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 |
https://hg.mozilla.org/mozilla-central/rev/270e22be3545
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
•