1937780 - 2.75% compiler_metrics num_static_constructors (Linux) regression on Mon December 16 2024

Status: RESOLVED WONTFIX

(Toolkit :: Gecko Trace, defect)

Description

[Treeherder Bug Filer]

Perfherder has detected a build_metrics performance regression from push 7510845c07dad11801f3959ea0a6d20e6a4d9195. 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%	compiler_metrics num_static_constructors	linux64		109.00 -> 112.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 patch(es) may be backed out in accordance with our regression policy.

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

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 2

[BugBot [:suhaib / :marco]]

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

:mvanstraten, since you are the author of the regressor, bug 1923143, 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 3

[Michael van Straten [:michael]]

I've tried to pinpoint the exact location of the new static constructors here but couldn't narrow it down to a unified object. I added some print statements in the script that analyzes the generated build symbols (here and here), but I couldn't cleanly link them back to any specific code locations.

Since we are vendoring a new third-party library here, I would argue that an increase would be tolerable. If not, :sergesanspaille, do you know how to correctly locate the static constructors?

Comment 4

[BugBot [:suhaib / :marco]]

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

:mvanstraten, since you are the author of the regressor, bug 1923143, 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 5

[[:sergesanspaille]]

Hey Michael,
we currently have a linter that should warn you whenever you add new global variables that don't have a constant initializer. It did not trigger on your patch which means that your patch does not add any global variable in mozilla code base. The linter does not trigger on third party, so it means you're adding global variables from third party (this is also my feeling from reading your patch).

From there you may want to contribute changes upstream, maybe there are some patterns that could be trivially changed, but according to me, it's only worth the effort if upstream is going to accept them, and I cannot answer this question for you.

Considering the actual constructors, you should look at the object file (.o) from those third party, then run nm object_file.o | grep _GLOBAL__sub. If there's one, then you have a dynamic global constructor. The -Wglobal-constructors compiler flag also helps, but it also warns for constructors that actually can be turned const.

Comment 6

[BugBot [:suhaib / :marco]]

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

:mvanstraten, since you are the author of the regressor, bug 1923143, 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 7

[BugBot [:suhaib / :marco]]

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

:mvanstraten, since you are the author of the regressor, bug 1923143, 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 8

[Michael van Straten [:michael]]

This was already backed out due to this push failure.