1939273 - 0.32% Base Content Resident Unique Memory (OSX) regression on Tue December 17 2024

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Treeherder Bug Filer]

Perfherder has detected a awsy performance regression from push 4bb5c3c364320f6bbf083ad979f151e1425e82f0. 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.32%	Base Content Resident Unique Memory	macosx1470-64-shippable	fission	15,683,669.33 -> 15,733,077.33

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 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.

You can run all of these tests on try with ./mach try perf --alert 43217

The following documentation link provides more information about this command.

For more information on performance sheriffing please see our FAQ.

If you have any questions, please do not hesitate to reach out to [e-mail].

Comment 1

[Tooru Fujisawa [:arai]]

This is expected, as mentioned in bug 1937701 comment #1 and #2

The size of AppConstants.sys.mjs increases by 6kB for each process, due to the line annotations.
The line annotations had been there when it was JSM, and it's accidentally removed while the ESModule migration, and those bugs revived them.

If the annotation isn't much beneficial, we can revisit the decision and possibly remove it from distributed files.

glandium, can I have your opinion?

Comment 2

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

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

Comment 3

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

It has been over 7 days with no activity on this performance regression.

:arai, since you are the author of the regressor, bug 1936088, which triggered this performance alert, could you please provide a progress update?

If this regression is something that fixes a bug, changes the baseline of the regression metrics, or otherwise will not be fixed, please consider closing it as WONTFIX. See this documentation for more information on how to handle regressions.

For additional information/help, please needinfo the performance sheriff who filed this alert (they can be found in comment #0), or reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1939273 - Reduce the size of preprocessed AppConstants.sys.mjs.

This deals with the easiest part, and cuts 5KB off the size of the
preprocessed file.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d979cd05c79e
Reduce the size of preprocessed AppConstants.sys.mjs. r=firefox-build-system-reviewers,nalexander

Comment 6

[amarc]

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

Comment 7

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

The patch landed in nightly and beta is affected.
:glandium, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox135 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Mike Hommey [:glandium]]

For some reason, this wasn't fixed by the patch that landed.

Comment 9

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

It has been over 7 days with no activity on this performance regression.

:arai, since you are the author of the regressor, bug 1936088, which triggered this performance alert, could you please provide a progress update?

If this regression is something that fixes a bug, changes the baseline of the regression metrics, or otherwise will not be fixed, please consider closing it as WONTFIX. See this documentation for more information on how to handle regressions.

For additional information/help, please needinfo the performance sheriff who filed this alert (they can be found in comment #0), or reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.

Comment 10

[Tooru Fujisawa [:arai]]

If the @line annotation is useful but only while debugging, how about adding a temporary file that contains the annotation, while not putting it in the final binary?

The preprocessed files are now reflected in the searchfox, and people can easily check the output there (of course in addition to the files in their own objdir).
https://searchfox.org/mozilla-central/search?q=AppConstants.sys.mjs&path=AppConstants.sys.mjs&case=true&regexp=false

We could for example have 2 output files for the preprocess step, one without the annotation for the final binary, and one with the annotation for the debugging purpose,
That way the extra text won't appear in the final binary.
Maybe it could be conditional on the filename extension (e.g. js and mjs), so that we don't have to perform unnecessary steps for files that are only temporary purpose.

This may also help some other preprocessed files that's included in the final binary, such as greprefs.js, where the annotation lines takes 7kB.

Comment 11

[Mike Hommey [:glandium]]

The more I look at this, the less I'm convinced it has anything to do with @lines. If I didn't botch things, even a patch that removes all of them without reverting bug 1936088 doesn't get us back to the memory usage from before bug 1936088. At this point, I think the increase in memory is due to the resulting change in how things end up ordered in the omni.ja. But I haven't had the time to check this further.

Comment 12

[Julien Wajsberg [:julienw]]

Hey glandium, can you please set the priority and severity? Thanks!

Comment 13

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

It has been over 7 days with no activity on this performance regression.

:arai, since you are the author of the regressor, bug 1936088, which triggered this performance alert, could you please provide a progress update?

If this regression is something that fixes a bug, changes the baseline of the regression metrics, or otherwise will not be fixed, please consider closing it as WONTFIX. See this documentation for more information on how to handle regressions.

For additional information/help, please needinfo the performance sheriff who filed this alert (they can be found in comment #0), or reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.

Comment 14

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

It has been over 7 days with no activity on this performance regression.

:arai, since you are the author of the regressor, bug 1936088, which triggered this performance alert, could you please provide a progress update?

If this regression is something that fixes a bug, changes the baseline of the regression metrics, or otherwise will not be fixed, please consider closing it as WONTFIX. See this documentation for more information on how to handle regressions.

For additional information/help, please needinfo the performance sheriff who filed this alert (they can be found in comment #0), or reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.

Comment 15

[Tooru Fujisawa [:arai]]

Moved the annotation part to bug 1949783.

and otherwise we can ignore the regression, as the regressor patch is virtually a part of bug 1881888's patch stack, and bug 1881888 comment #22's improvement should compensate the regression here.
I'll close this as WONTFIX.

Comment 16

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

A comment containing a reason for why the performance regression was resolved as WONTFIX could not be found. It should provided when the status of the bug is changed.

:arai, since you resolved this bug, could you provide a comment explaining the resolution? If one has already been provided, this needinfo can be ignored/removed.

If you need additional information/help, reach out in #perftest, or #perfsheriffs on Element.

For more information, please visit BugBot documentation.