1601561 - 0.98% compiler_metrics num_static_constructors (android-5-0-x86_64) regression on push fba17efeb06c0e58af3bc11c7bd3b82fbfd89def (Sun November 24 2019)

Status: RESOLVED FIXED

(Testing :: Performance, defect)

Description

[Alexandru Ionescu (needinfo me) [:alexandrui]]

We have detected a build metrics regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=fba17efeb06c0e58af3bc11c7bd3b82fbfd89def

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

1% compiler_metrics num_static_constructors android-5-0-x86_64 opt 85.17 -> 86.00

You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=24266

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Comment 1

[Alexandru Ionescu (needinfo me) [:alexandrui]]

The downstream caused regression higher than 2%. Looks like the autoland recovered from this thanks to Bug 1560664, but beta didn't. Let's wait until beta recovers before closing this.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Odds are somewhere between slim and none that we're going to do anything for Beta at this point if the regression is gone on m-c. That said, I'm not entirely sure that this was an expected outcome from bug 1560664 either.

Comment 3

[:dmajor]

That said, I'm not entirely sure that this was an expected outcome from bug 1560664 either.

Different constructors.

Bug 1570549 added a new constructor at _GLOBAL__sub_I_RsdparsaSdpParser.cpp, most likely WEBRTC_SDP_NAME.

Bug 1560664 removed a constructor at _GLOBAL__sub_I_Unified_cpp_accessible_base1.cpp, I didn't bother checking which.

Comment 4

[:dmajor]

Since this wasn't exactly fixed, more like masked, I'm reopening.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Thanks for digging into that, David. Probably worth seeing what we can do about that new webrtc constructor then.

Comment 6

[Nico Grunbaum [:ng, @chew:mozilla.org]]

I can look at this today.

Comment 7

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1601561 - Move parser names to static functions.;r?dminor

Comment 8

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0cedf9a8ec8db30fc441bb76bf82273685bce7a7

Comment 9

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 9115796 [details]
Bug 1601561 - Move parser names to static functions.;r?dminor

Beta/Release Uplift Approval Request

• User impact if declined: Regression in static constructors leading to potentially slower startup times.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This code moves some static strings into static member functions. If there is time, it can bake a day on Nightly.
• String changes made/needed:

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b194872cda4b
Move parser names to static functions.;r=dminor

Comment 11

[Oana Pop-Rus]

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

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9115796 [details]
Bug 1601561 - Move parser names to static functions.;r?dminor

let's get this one in beta for 72.0b9

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2cd74ee9acf0