1727877 - 3.2 - 3.17% Base Content Heap Unclassified / Base Content Heap Unclassified (Linux) regression on Wed August 25 2021

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P1)

Description

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a awsy performance regression from push c8933e49a64bb79e25d333e7c832880048cb5712. 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)
3%	Base Content Heap Unclassified	linux1804-64-shippable-qr	fission	2,062,059.83 -> 2,127,969.67
3%	Base Content Heap Unclassified	linux1804-64-shippable-qr	fission	2,062,438.67 -> 2,127,772.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) will be backed out in accordance with our regression policy.

For more information on performance sheriffing please see our FAQ.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you for the report.
My guess is that this is due to this patch, which is allocating a buffer in each thread that has a JSContext (i.e., it's running Javascript).
This helps with lowering the overhead of profiling, by not having to share a single buffer when working with mutliple Javascript runners (main thread and workers).

It should be possible to only allocate this buffer when the user is actually profiling; I didn't do it from the start, because it added complexity, and may reintroduce some unwanted locking... I will investigate next week.

Comment 2

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

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

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1727877 - Only allocate JS frame buffer when thread is being profiled - r?canaltinova

The buffer was previously allocated as soon as a JSContext was present, meaning that some memory was used even when the profiler was not running, which is the case most of the time for most users.
This saves some memory, at the cost of having to lock the per-thread ThreadRegistration data when sampling a thread with a JSContext. This should have low contention risk, because it's mostly used when sampling on the thread, while the periodic sampler uses its own buffer so it doesn't need to lock the per-thread data.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Some results: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=281364c43774d44fbd33d953e4e988f9299afdd1&newProject=try&newRevision=eaf8449611a9bd7c3b04ee52d9f3629d908d83e2&framework=4&page=1&filter=base
E.g.:
Base Content Heap Unclassified opt fission: 2,127,347 -> 2,061,988 -3.07%

So it confirms that the always-allocated JS frame buffer was the issue, the attached patch reverses that; now the buffer is only allocated when profiling actually starts.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6242f4c09c5
Only allocate JS frame buffer when thread is being profiled - r=canaltinova

Comment 6

[Alexandru Michis [:malexandru]]

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

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

The drop back to the previous values is visible in perfherder, starting at the push linked in comment 6:
https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&series=autoland,3806443,1,4&timerange=1209600&zoom=1629807456334,1630498795272,2032887.6725662716,2175644.8430400826

Comment 8

[Acasandrei Beatrice (needinfo me)]

== Change summary for alert #31132 (as of Fri, 03 Sep 2021 04:51:05 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	Base Content Heap Unclassified	linux1804-64-shippable-qr	fission	2,125,948.33 -> 2,059,764.33
3%	Base Content Heap Unclassified	linux1804-64-shippable-qr	fission	2,125,094.83 -> 2,059,624.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31132