Closed Bug 1601561 Opened 2 years ago Closed 2 years ago

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

Categories

(Testing :: Performance, defect)

Version 3
defect
Not set
normal

Tracking

(firefox-esr68 unaffected, firefox71 unaffected, firefox72 fixed, firefox73 fixed)

RESOLVED FIXED
mozilla73
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- unaffected
firefox72 --- fixed
firefox73 --- fixed

People

(Reporter: alexandrui, Assigned: ng)

References

(Regression)

Details

(Keywords: perf-alert, regression)

Attachments

(1 file)

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! ***

Flags: needinfo?(na-g)

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.

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.

Blocks: 1592626
No longer blocks: 1600879
Status: NEW → RESOLVED
Closed: 2 years ago
Depends on: cxx17
Flags: needinfo?(na-g)
Resolution: --- → FIXED

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.

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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

I can look at this today.

Assignee: nobody → na-g

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:
Flags: needinfo?(na-g)
Attachment #9115796 - Flags: approval-mozilla-beta?
Pushed by rvandermeulen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b194872cda4b
Move parser names to static functions.;r=dminor
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73

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

Attachment #9115796 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.