1943696 - Add JitDump support for IR with Wasm Ion

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P3)

Description

[Ryan Hunt [:rhunt]]

With samply and jit dump support [1] we can get nice profiles that show the disassembly for wasm functions. It also has support for showing the IR operations for functions, but we don't support it in wasm. It looks like we need to call 'saveProfile' [2] and it might just work for Ion. Baseline would need special support, so it can be a separate bug.

[1] bit.ly/jit-profiling
[2] https://searchfox.org/mozilla-central/rev/5efa5bfe9c217f6ab3529880b25a52ac6405dda8/js/src/jit/PerfSpewer.h#85

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 1943696 - wasm: Add JitDump support for optimizing code. r?iain

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1943696 - wasm: Add JitDump support for baseline code. r?iain

Same as the previous patch, this time re-using more code.

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1943696 - wasm: Fix bug in offset calculation for JitDump. r?iain

Our JitDump integration uses our CodeRange metadata to find the
start of the generated function (see CodeBlock::sendToProfiler).

However, all the code using PerfSpewer records offsets using
masm.currentOffset(), and we also compile functions in batches
re-using the same MacroAssembler. This leads to the offsets
stored in PerfSpewer being relative to the batch, not the
individual function. Fix this by adding a subtraction and
markStartOffset method.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbb992cdb35f
wasm: Add JitDump support for optimizing code. r=iain
https://hg.mozilla.org/integration/autoland/rev/89bdbfa6aaac
wasm: Add JitDump support for baseline code. r=iain
https://hg.mozilla.org/integration/autoland/rev/ac6c8a585500
wasm: Fix bug in offset calculation for JitDump. r=iain
https://hg.mozilla.org/integration/autoland/rev/f3bcc54d34e3
apply code formatting via Lando

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/fbb992cdb35f
https://hg.mozilla.org/mozilla-central/rev/89bdbfa6aaac
https://hg.mozilla.org/mozilla-central/rev/ac6c8a585500
https://hg.mozilla.org/mozilla-central/rev/f3bcc54d34e3

Comment 6

[Alex Finder]

(In reply to Pulsebot from comment #4)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbb992cdb35f
wasm: Add JitDump support for optimizing code. r=iain
https://hg.mozilla.org/integration/autoland/rev/89bdbfa6aaac
wasm: Add JitDump support for baseline code. r=iain
https://hg.mozilla.org/integration/autoland/rev/ac6c8a585500
wasm: Fix bug in offset calculation for JitDump. r=iain
https://hg.mozilla.org/integration/autoland/rev/f3bcc54d34e3
apply code formatting via Lando

Hi Ryan !
Is it possible that the push referenced in comment 4 may have caused the performance regression mentioned below ?
Perfherder has detected a mozperftest performance regression from push fbb992cdb35f5c71c60a961e8f2f85e6351c4ef8. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	browser_ml_autofill_perf.js AUTOFILL-pipeline-ready-latency	linux1804-64-shippable		489.58 -> 502.88

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 43836

The following documentation link provides more information about this command.

For more information on performance sheriffing please see our FAQ.

If you have any questions, please do not hesitate to reach out to [e-mail].

Comment 7

[Ryan Hunt [:rhunt]]

It seems likely that this patch caused the regression. I filed bug 1949824 to land a fix. As noted in the comment on the bug, this should only affect nightly.