1815175 - 0.26 - 0.25% Base Content JS / Base Content JS (OSX) regression on Wed February 1 2023

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P1)

Description

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a awsy performance regression from push 5b90fc9fed427a15a6cbc8466b612a30cd3c5f0b. 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)
0.26%	Base Content JS	macosx1015-64-shippable-qr	fission	1,631,722.67 -> 1,635,914.67
0.25%	Base Content JS	macosx1015-64-shippable-qr	fission	1,631,785.33 -> 1,635,886.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

[Jan de Mooij [:jandem]]

smaug, do you know what's the size of the cache? I thought it was relatively small.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

The cache is 31 * sizeof(void*), so that shouldn't have increased the number too much.

Is the regression possibly coming from some other bug? Are there other suspicious looking changes around?

Though, I guess the cache might change GC or mozjemalloc behavior a bit since it decreases how many allocations we do.

Comment 3

[Jan de Mooij [:jandem]]

You could compare the sizeof of the data structure it's embedded in. Maybe it now needs a larger jemalloc bucket or something.

Comment 4

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

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

Comment 5

[Acasandrei Beatrice (needinfo me)]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #2)

The cache is 31 * sizeof(void*), so that shouldn't have increased the number too much.

Is the regression possibly coming from some other bug? Are there other suspicious looking changes around?

Though, I guess the cache might change GC or mozjemalloc behavior a bit since it decreases how many allocations we do.

The graph is pretty stable. I don't believe the regression could have been caused by another bug.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Ok.
anyhow, the performance boost was so significant that I think we should just take this minor change to memory numbers.

Comment 7

[Ted Campbell [:tcampbell]]

(In reply to Jan de Mooij [:jandem] from comment #3)

You could compare the sizeof of the data structure it's embedded in. Maybe it now needs a larger jemalloc bucket or something.

about:memory diff shows change is in js-non-window/runtime/runtime-object @ 4kB per process due to jemalloc chunks as Jan suggested.

Sizes for linux opt builds have the expected 248 B change for the small cache.

Before patch
(gdb) p sizeof(js::StringToAtomCache)
$1 = 56
(gdb) p sizeof(js::RuntimeCaches)
$2 = 33048
(gdb) p sizeof(JSRuntime)
$3 = 44744

After patch
(gdb) p sizeof(js::StringToAtomCache)
$1 = 304
(gdb) p sizeof(js::RuntimeCaches)
$2 = 33296
(gdb) p sizeof(JSRuntime)
$3 = 44992

Regression is only on OSX and the OSX number had an unusual 4kb movement earlier so this matches the jemlloc bucket theory.
https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&series=autoland,3806445,1,4&series=autoland,3807462,1,4&timerange=2592000&zoom=1673309559063,1675911084463,1607227.5555555562,1646916.4444444426

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Ted Campbell [:tcampbell] from comment #7)

(In reply to Jan de Mooij [:jandem] from comment #3)

You could compare the sizeof of the data structure it's embedded in. Maybe it now needs a larger jemalloc bucket or something.

about:memory diff shows change is in js-non-window/runtime/runtime-object @ 4kB per process due to jemalloc chunks as Jan suggested.

Sizes for linux opt builds have the expected 248 B change for the small cache.

Thanks for confirming this! It's good to understand what's going on here.