1890247 - 41.38 - 2.52% reload-debugger:content-process objects-with-stacks / damp cold.jsdebugger.open.DAMP + 25 more (Linux, Windows) regression on Tue April 2 2024

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

Perfherder has detected a devtools performance regression from push 52bb75377d06cfb305446ca966c7526c9c9729c7. 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)
41%	reload-debugger:content-process objects-with-stacks	linux1804-64-qr		986.00 -> 1,394.00
41%	reload-debugger:content-process objects-with-stacks	linux1804-64-shippable-qr		986.00 -> 1,394.00
41%	reload-debugger:content-process objects-with-stacks	linux1804-64-tsan-qr		986.00 -> 1,394.00
40%	reload-webconsole:content-process objects-with-stacks	linux1804-64-tsan-qr		1,000.50 -> 1,396.00
39%	reload-webconsole:content-process objects-with-stacks	linux1804-64-qr		1,001.00 -> 1,396.00
39%	reload-webconsole:content-process objects-with-stacks	linux1804-64-shippable-qr		1,001.00 -> 1,396.00
39%	reload-netmonitor:content-process objects-with-stacks	linux1804-64-qr		1,006.00 -> 1,400.00
39%	reload-netmonitor:content-process objects-with-stacks	linux1804-64-shippable-qr		1,006.00 -> 1,400.00
39%	reload-netmonitor:content-process objects-with-stacks	linux1804-64-tsan-qr		1,006.00 -> 1,400.00
26%	reload-debugger:content-process memory	linux1804-64-qr		2,716,672.00 -> 3,417,088.00
...	...	...	...	...
3%	damp inspector.layout.open	windows10-64-shippable-qr	e10s fission stylo webrender	218.94 -> 225.51
3%	damp cold.inspector.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	317.36 -> 326.52
3%	damp cold.webconsole.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	327.92 -> 337.41
3%	damp simple.webconsole.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	243.91 -> 250.85
3%	damp cold.jsdebugger.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	470.37 -> 482.22

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
35%	damp simple.styleeditor.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	5.91 -> 3.85
34%	damp simple.styleeditor.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	5.80 -> 3.82
30%	damp simple.netmonitor.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	9.19 -> 6.47
28%	damp simple.webconsole.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	10.84 -> 7.80
28%	damp simple.netmonitor.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	9.09 -> 6.57
...	...	...	...	...
3%	damp console.objectexpanded.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	55.11 -> 53.32

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 42098

For more information on performance sheriffing please see our FAQ.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1866814

Comment 2

[Donal Meehan [:dmeehan]]

I think ochameau is out this week.
We are in the final week of lightly before Fx1256 goes to beta next week.
:nchevobbe, any idea of the Severity on this?

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

I don't think it's too bad if this gets shipped

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1890247 - [devtools] Prevent leaking WindowGlobal target actor via LazyPool.

We shouldn't try to request target actor's form late during the destroy sequence.
This would force to re-create the Lazy Target Scoped Actors via LazyPool,
which would ultimately force to re-instantiate a new _targetScopedActorsPool
which wouldn't be destroyed as it is created post-destruction.

Instead, only pass a partial form, with only the actorID,
it is enough for protocol.js on the client side to match an existing Front instance.

Comment 5

[Alexandre Poirot [:ochameau]]

We may actually have a sizeable leak on page reload which would be worth uplifting to 126.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b453de1f5c2c
[devtools] Prevent leaking WindowGlobal target actor via LazyPool. r=devtools-reviewers,nchevobbe

Comment 7

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/b453de1f5c2c

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:ochameau, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Leaks fix is reported on perfherder (and we get even lower numbers than before the regression)

Perfherder has detected a devtools performance change from push 57f6925a520cf97f4af35e42db00435e817475dc.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
35%	reload-webconsole:content-process objects-with-stacks	linux1804-64-qr		1,396.00 -> 907.00
35%	reload-netmonitor:content-process objects-with-stacks	linux1804-64-qr		1,400.00 -> 912.00
34%	reload-debugger:content-process objects-with-stacks	linux1804-64-qr		1,394.00 -> 915.00
17%	reload-netmonitor:content-process memory	linux1804-64-qr		2,182,387.81 -> 1,802,922.67
17%	reload-webconsole:content-process memory	linux1804-64-qr		2,268,856.32 -> 1,880,405.33
11%	reload-inspector:content-process objects-with-stacks	linux1804-64-qr		4,431.00 -> 3,942.00

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

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 42294

For more information on performance sheriffing please see our FAQ.

Comment 10

[Alexandre Poirot [:ochameau]]

Comment on attachment 9396757 [details]
Bug 1890247 - [devtools] Prevent leaking WindowGlobal target actor via LazyPool.

Beta/Release Uplift Approval Request

• User impact if declined: Significant memory leak when reloading pages while having devtools opened.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple bailout in one method.
• String changes made/needed:
• Is Android affected?: No

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9396757 [details]
Bug 1890247 - [devtools] Prevent leaking WindowGlobal target actor via LazyPool.

Approved for 126.0b3

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/0e714df00360