1916533 - Always use native backend when recording JS Traces to the profiler output

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Alexandre Poirot [:ochameau]]

Bug 1906719 introduce a much faster implementation for recording JS traces to the profiler.
This is still behind a preference, but I think it could become the default and only one implementation.
Everyone would benefit from the performance improvement.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1916533 - [devtools] Always use native codepath when recording JS traces to the profiler output.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e113ea902ab
[devtools] Always use native codepath when recording JS traces to the profiler output. r=devtools-reviewers,nchevobbe

Comment 3

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/2e113ea902ab

Comment 4

[Nicolas Chevobbe [:nchevobbe][PTO - back on October 21]]

Alex, I'm seeing an alert that could be pointing to this change, was this expected?

---

Perfherder has detected a devtools performance change from push 91431bf2cb698a8467ce65c675b621dc30a13f7e.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
130%	damp jstracer.profiler-collection-performance.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	295.82 -> 679.77
118%	damp jstracer.profiler-collection-performance.DAMP	windows11-64-shippable-qr	e10s fission stylo webrender	165.99 -> 361.79
99%	damp jstracer.profiler-collection-performance.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	432.58 -> 859.58
6%	damp simple.jsdebugger.reload.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	222.71 -> 235.88

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
92%	damp jstracer.profiler-recording-performance.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	1,883.92 -> 146.63
91%	damp jstracer.profiler-recording-performance.DAMP	windows11-64-shippable-qr	e10s fission stylo webrender	909.62 -> 78.13
88%	damp jstracer.profiler-recording-performance.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	2,504.41 -> 296.62
15%	damp complicated.jsdebugger.close.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	24.19 -> 20.63
13%	damp complicated.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	18.14 -> 15.78
4%	damp simple.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	10.62 -> 10.15

As author of one of the patches included in that push, we need your help to address this regression.
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 these tests on try with ./mach try perf --alert 1991

For more information on performance sheriffing please see our FAQ.

Comment 5

[Alexandre Poirot [:ochameau]]

Yes this performance change was expected.

The native collection of traces made the "recording" step much faster (jstracer.profiler-recording-performance).
i.e. when you are executing some JS and recording in background.

But the native collection also does more work at "collection" step (jstracer.profiler-collection-performance).
i.e. when you stop recording and the native implementation has to build the profiler JSON data format,
which in the JS-implementation was crafter over time as JS was traced.

We have a 10x+ improvement on recording in exchange of a 2-3x+ regression on collection.