1743423 - 0.92 - 0.9% Base Content JS / Base Content JS + 2 more (Linux, OSX, Windows) regression on Fri November 26 2021

Status: RESOLVED WONTFIX

(Core :: JavaScript: GC, defect, P3)

Description

[Andrej (:aglavic)]

Perfherder has detected a awsy performance regression from push 0daeadd3c56cce26b351c7cc3b067d0dda71ecb4. 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)
1%	Base Content JS	linux1804-64-shippable-qr		1,786,718.00 -> 1,803,140.67
1%	Base Content JS	linux1804-64-shippable-qr		1,786,756.67 -> 1,803,188.67
1%	Base Content JS	windows10-64-2004-shippable-qr		1,792,120.00 -> 1,808,504.00
1%	Base Content JS	macosx1015-64-shippable-qr		1,813,760.00 -> 1,830,168.00

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.

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 1742591

Comment 2

[Jon Coppeard (:jonco)]

(In reply to Andrej (:andrej) from comment #0)
This is a 16KB increase on all platforms. On the plus side, preliminary telemetry results show a 20% reduction in time spent in the root marking phase. However that phase is usually pretty short (median 400 uS, although it can be much longer). The change is also a decent code simplification.

Comment 3

[Jon Coppeard (:jonco)]

I'd like to keep the code simplification. It's possible we could reduce the memory overhead by storing the pinned atoms in a sparse bitmap like we do for Zone::markedAtoms_. I'll test a patch that does that.

Comment 4

[Jon Coppeard (:jonco)]

Jan also has a patch which we hope will mitigate most of this increase.

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #3)
Storing pinned atoms in a sparse bitmap improves base content JS by 7KB.

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #4)
Bug 1744036 has improved the situation by saving 10KB, mitigating a lot of this regression. I think we should accept the remaining 6KB increase.

Hopefully future changes will allow us to statically allocate all these atoms and improve things further.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Are we planning to uplift bug 1744036 to Beta to mitigate the regression in 96 too?

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)

Are we planning to uplift bug 1744036 to Beta to mitigate the regression in 96 too?

It's not a large memory usage regression and the XPConnect changes are not trivial, so I'd prefer letting it ride the trains in case it regressed something.