1833078 - 2.02% Base Content Heap Unclassified (Windows) regression on Mon May 8 2023

Status: ASSIGNED

(Core :: JavaScript Engine, defect, P3)

Description

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a awsy performance regression from push 764a0722b38f6bf15121a772f301221a394d384c. 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)
2%	Base Content Heap Unclassified	windows11-64-2009-shippable-qr	fission	1,665,193.33 -> 1,698,871.33

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
1%	Base Content JS	windows11-64-2009-shippable-qr	fission	1,592,545.33 -> 1,579,714.00
1%	Base Content JS	linux1804-64-shippable-qr	fission	1,591,217.67 -> 1,579,966.67
1%	Base Content JS	macosx1015-64-shippable-qr	fission	1,591,301.33 -> 1,580,024.00
0.34%	Base Content JS	windows11-64-2009-shippable-qr	fission	1,584,202.13 -> 1,578,892.67

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) 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.

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 1828496

Comment 2

[Jan de Mooij [:jandem]]

"Heap Unclassified" suggests we should probably add a memory reporter.

Comment 3

[Alex Thayer [:alexical] (she/her)]

Sorry, I will take this, but to be clear this is expected and intentional. Two things happened here - A) we increased the size of the MegamorphicSetPropCache, which gave us wins on various speedometer 3 subbenchmarks, and B) we moved the cache to be allocated on the heap because of Reasons. If we think it's worth it, I can add a custom memory allocator for the MegamorphicSetPropCache, but it should always report the same statically knowable value. Other than that I don't think there's anything to do here.

Comment 4

[Jan de Mooij [:jandem]]

It doesn't have to be a new memory reporter, you could also add it to the reporter that we used for it before. At least that moves it out of heap-unclassified which is random stuff that's hard to track.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Adjusting priority; P1/S4 doesn't quite make sense.