1737602 - 0.28 - 0.28% Base Content JS / Base Content JS + 2 more (Linux, OSX, Windows) regression on Thu October 21 2021

Status: RESOLVED WONTFIX

(WebExtensions :: General, defect)

Description

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a awsy performance regression from push 90f87be213acc10edfbb93c75f2da0f05bf7df2d. 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.28%	Base Content JS	linux1804-64-shippable-qr		1,780,240.00 -> 1,785,284.67
0.28%	Base Content JS	macosx1015-64-shippable-qr		1,782,928.00 -> 1,787,984.00
0.28%	Base Content JS	windows10-64-2004-shippable-qr		1,785,872.00 -> 1,790,920.00
0.28%	Base Content JS	macosx1015-64-shippable-qr		1,782,928.00 -> 1,787,992.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.

Comment 1

[Shane Caraveo (:mixedpuppy)]

I'm not sure what the numbers represent "1,782,928.00 -> 1,787,984.00" so it's hard to know how much of a memory impact this is. Merely adding a builtin addon here is going to have overhead. So we need some time (and someone who understands what the regression represents) to evaluate what can be done. Then we'd need time to make those changes (if some are possible).

I guess my question is can we live with this regression for a cycle to give us time to figure this out?

Comment 2

[Acasandrei Beatrice (needinfo me)]

Take your time to investigate. You can also take a look at the documentation to get some further information.

Comment 3

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

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

Comment 4

[Shane Caraveo (:mixedpuppy)]

(In reply to Acasandrei Beatrice from comment #2)

Take your time to investigate. You can also take a look at the documentation to get some further information.

Yeah, I looked at that, but it doesn't mention what the values represent. Are they bytes of memory?

Comment 5

[Acasandrei Beatrice (needinfo me)]

Yes, the measurement unit is bytes. When in doubt about the measurement unit you can check the Graphs View page in Perfherder

Comment 6

[Tomislav Jovanovic :zombie]

Search detection is an important security feature, that we're continuing to work on. We have a plan to land this as part of Firefox code (not as an addon), and we believe this will alleviate the memory regression, but we're not ready to do that just yet. For these reasons, (speaking as a WebExtensions module owner) I'm willing to accept this (small) regression for the time being.

Please note that this doesn't mean we don't take performance and memory efficiency seriously. Just in the last 2 months, we've landed several significant improvements to Base JS that are an order of magnitude larger than this regression (5-6% Base Content JS memory reduction from bug 1708193 and bug 1708243).

If you have any questions, feel free to ask here or contact me directly.

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Acasandrei Beatrice from comment #5)

Yes, the measurement unit is bytes. When in doubt about the measurement unit you can check the Graphs View page in Perfherder

Beatrice can you open a ticket for including the units in alerts view and bugzilla comments? We should also add this to the test documentation.