3.2 - 3.17% Base Content Heap Unclassified / Base Content Heap Unclassified (Linux) regression on Wed August 25 2021
Categories
(Core :: Gecko Profiler, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox-esr91 | --- | unaffected |
firefox91 | --- | unaffected |
firefox92 | --- | unaffected |
firefox93 | --- | fixed |
People
(Reporter: bacasandrei, Assigned: mozbugz)
References
(Regression)
Details
(Keywords: perf, perf-alert, regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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.
Assignee | ||
Comment 1•3 years ago
|
||
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•3 years ago
|
||
Set release status flags based on info from the regressing bug 1716959
Assignee | ||
Comment 3•3 years ago
|
||
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.
Assignee | ||
Comment 4•3 years ago
|
||
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 6•3 years ago
|
||
bugherder |
Assignee | ||
Comment 7•3 years ago
|
||
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
Reporter | ||
Comment 8•3 years ago
|
||
== 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
Updated•3 years ago
|
Description
•