1773984 - 12.14 - 2.25% damp simple.jsdebugger.close.DAMP / damp custom.inspector.close.DAMP + 31 more (OSX) regression on Tue June 7 2022

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect, P3)

Description

[Julian Descottes [:jdescottes]]

Perfherder has detected a devtools performance regression from push 804d4cc1ed3766d5a70a8a914c3721bb23aee816. 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)
12%	damp simple.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	11.22 -> 12.58
10%	damp simple.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	11.30 -> 12.45
10%	damp simple.netmonitor.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	6.53 -> 7.17
9%	damp simple.styleeditor.reload.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	19.00 -> 20.78
9%	damp simple.netmonitor.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	6.55 -> 7.15
9%	damp complicated.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	22.70 -> 24.65
8%	damp custom.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	24.88 -> 26.81
8%	damp custom.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	24.97 -> 26.91
8%	damp custom.netmonitor.reload.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	239.81 -> 258.26
8%	damp custom.netmonitor.reload.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	240.30 -> 258.51
...	...	...	...	...
3%	damp console.log-in-loop-content-process-bool	macosx1015-64-shippable-qr	e10s fission stylo webrender	36.30 -> 37.44
3%	damp console.log-in-loop-content-process-symbol	macosx1015-64-shippable-qr	e10s fission stylo webrender	48.05 -> 49.48
3%	damp console.log-in-loop-content-process-typedarray	macosx1015-64-shippable-qr	e10s fission stylo webrender	72.84 -> 74.93
3%	damp inspector.layout.open	macosx1015-64-shippable-qr	e10s fission stylo webrender	236.07 -> 242.29
2%	damp custom.inspector.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	44.72 -> 45.72

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 offending patch(es) will 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.

For more information on performance sheriffing please see our FAQ.

Comment 1

[Julian Descottes [:jdescottes]]

Widespread macos regression on DevTools performance tests, most likely related to Bug 1670885.

Comment 2

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

The Bugbug bot thinks this bug is a defect, but please change it back in case of error.

Comment 3

[Gabriele Svelto [:gsvelto]]

Alright, this isn't completely unexpected: the new locks we're using aren't spin-locks, they go straight to the kernel in case of contention. This is probably the root cause of the regression. I know there should be some tunables around this locks so I'll experiment with them until I find a solution that addresses these regressions.

Comment 4

[Julian Descottes [:jdescottes]]

(In reply to Gabriele Svelto [:gsvelto] from comment #3)

Alright, this isn't completely unexpected: the new locks we're using aren't spin-locks, they go straight to the kernel in case of contention. This is probably the root cause of the regression. I know there should be some tunables around this locks so I'll experiment with them until I find a solution that addresses these regressions.

Thanks for taking a look! I'm moving the bug over to the component of the original bug.
However this is not a huge performance regression, so if we need to accept this as the new baseline, just let us know. Thanks!

Comment 5

[Gabriele Svelto [:gsvelto]]

(In reply to Julian Descottes [:jdescottes] from comment #4)

Thanks for taking a look! I'm moving the bug over to the component of the original bug.
However this is not a huge performance regression, so if we need to accept this as the new baseline, just let us know. Thanks!

In other tests it's rather bad so I'm looking for ways to mitigate it. We're a bit in a rock-and-a-hard-place situation here because the old deprecated user-space spinlocks were "fast" but only on an unloaded machine, as the load climbed they'd degrade very badly and cause live-locks on ARM-based macs. They were also a potential power hog in contested scenarios. The new locks aren't spinlocks so in lightly contested scenarios they're markedly worse - as the thread trying to grab one always goes to sleep even when the lock will be available in a very brief amount of time.

Comment 6

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

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

Comment 7

[Donal Meehan [:dmeehan]]

Could this be assigned since it's under investigation?

Comment 8

[Gabriele Svelto [:gsvelto]]

Sure! I've got a fix waiting for review in bug 1774458.

Comment 9

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

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

Comment 10

[Donal Meehan [:dmeehan]]

Setting 103 and 104 to Fixed, bug 1774458 landed in central and was uplifted to beta.