6.12 - 3.16% reload-debugger:parent-process objects-with-no-stacks / damp custom.jsdebugger.project-search.DAMP + 2 more (Linux, OSX, Windows) regression on Tue November 16 2021
Categories
(DevTools :: Debugger, defect, P3)
Tracking
(firefox-esr91 unaffected, firefox94 unaffected, firefox95 unaffected, firefox96 affected)
| Tracking | Status | |
|---|---|---|
| firefox-esr91 | --- | unaffected |
| firefox94 | --- | unaffected |
| firefox95 | --- | unaffected |
| firefox96 | --- | affected |
People
(Reporter: jdescottes, Unassigned)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: perf, perf-alert, regression)
Perfherder has detected a devtools performance regression from push bb05a9c04907014611396dec1fbfe65b0eba8295. 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) |
|---|---|---|---|---|
| 6% | reload-debugger:parent-process objects-with-no-stacks | linux1804-64-shippable-qr | 2,956.83 -> 3,137.92 | |
| 5% | damp custom.jsdebugger.project-search.DAMP | macosx1015-64-shippable-qr | e10s stylo webrender | 1,278.82 -> 1,344.84 |
| 5% | damp custom.jsdebugger.project-search.DAMP | macosx1015-64-shippable-qr | e10s stylo webrender-sw | 1,278.78 -> 1,339.66 |
| 3% | damp custom.jsdebugger.project-search.DAMP | windows10-64-shippable-qr | e10s stylo webrender-sw | 1,311.27 -> 1,352.76 |
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|
| 16% | reload-debugger:parent-process objects-with-stacks | linux1804-64-shippable-qr | 61.00 -> 51.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.
|
Reporter | |
Comment 1•3 years ago
|
||
Hi Hubert!
This performance regression is quite small, but it seems clearly related to Bug 1736366, and I saw some performance-related threads in the review at https://phabricator.services.mozilla.com/D128742#inline-707471. So in case you want to revisit this and try to improve the performance, it should get picked up by DAMP. Otherwise, feel free to close it, considering the performance change is quite small.
|
|
Reporter | |
Updated•3 years ago
|
|
||
Comment 2•3 years ago
|
||
Set release status flags based on info from the regressing bug 1736366
|
||
Comment 3•3 years ago
|
||
Thanks for the ping on this.
(In reply to Julian Descottes [:jdescottes] from comment #1)
Hi Hubert!
This performance regression is quite small, but it seems clearly related to Bug 1736366, and I saw some performance-related threads in the review at https://phabricator.services.mozilla.com/D128742#inline-707471.
I already applied those suggestions in the thread, we are no longer using lodash.
So in case you want to revisit this and try to improve the performance, it should get picked up by DAMP. Otherwise, feel free to close it, considering the performance change is quite small.
So i actually expected a little perf regression relating to the bug. It's probably from the fact that the patch stop using queryCacheShallow from here https://searchfox.org/mozilla-central/rev/702199bca53babc925e47fd8f86ed56487d42492/devtools/client/debugger/src/utils/resource/query-cache.js#29. The goal of the patch was to move away from this really the complex resource logic layer, and the tradeoff was between the complexity of the layer vs the amount perf regession.
I would say since it's small, i'm ok with it for now.
|
|
Reporter | |
Comment 4•3 years ago
|
||
Thanks for the update Bomsy! Closing as wontfix since this change was an expected tradeoff to simplify the implementation.
|
||
Updated•3 years ago
|
Description
•