Closed
Bug 1361383
Opened 8 years ago
Closed 8 years ago
Enable asm spew only in JS_JITSPEW builds
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(1 file)
2.36 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
For each instruction we emit on x86/x64 we check this:
if (MOZ_UNLIKELY(printer || JitSpewEnabled(JitSpew_Codegen))) {
va_list va;
va_start(va, fmt);
spew(fmt, va);
va_end(va);
}
JitSpewEnabled is a no-op in opt builds but we still have to null-check |printer| and the spew() call is likely messing with inlining heuristics.
Considering only a handful of people use this, let's only enable it in DEBUG/--enable-jitspew builds.
Attachment #8863757 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Comment 1•8 years ago
|
||
I wouldn't be surprised if this is actually a measurable speedup with Wasm Baseline.
Comment 2•8 years ago
|
||
Seems plausible. It hasn't shown up in profiles, but if it's inline it wouldn't, and anyway I haven't profiled in a while...
Comment 3•8 years ago
|
||
Comment on attachment 8863757 [details] [diff] [review]
Patch
Review of attachment 8863757 [details] [diff] [review]:
-----------------------------------------------------------------
This will might cause issues with the Ion counter, used by -D, and one addon, but this patch sounds fine to me.
Note: The ARM backend also has a printer_ field used by the disassembler/simulator.
Attachment #8863757 -
Flags: review?(nicolas.b.pierron) → review+
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #3)
> This will might cause issues with the Ion counter, used by -D, and one
> addon, but this patch sounds fine to me.
Yup, I think we can ask the few people using these to use an --enable-jitspew build.
> Note: The ARM backend also has a printer_ field used by the
> disassembler/simulator.
Yeah that one is already guarded by JS_DISASM_ARM (disabled by default in opt builds as far as I can see).
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/270e22be3545
Enable x86/x64 asm spew only in JS_JITSPEW builds. r=nbp
Comment 6•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•