1504122 - 4.19 - 6.15% compiler_metrics num_static_constructors (windows2012-64) regression on push b31fa37d257e44f611fc2e778aad242f97c2f1ec (Thu Nov 1 2018)

Status: RESOLVED WORKSFORME

(Core :: JavaScript: Internationalization API, defect)

Description

[Ionuț Goldan [:igoldan]]

We have detected a build metrics regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=9c00b907189039fdf10b0b3f431f65e2da5e1687

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

Regressions:

  6%  compiler_metrics num_static_constructors windows2012-64 opt msvc     585.00 -> 621.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=17329

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

Comment 1

[Ionuț Goldan [:igoldan]]

:anba for a very short while, builds on Windows 2012 x64 opt platform broke [1]. When they got fixed, we noticed this regression. Out of the 3 suspect bugs, namely bug 1499026, bug 1503671 and bug 1473588, I concluded that bug 1499026 is more related to this issue, given the C++ changes.

[1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&group_state=expanded&tochange=9c00b907189039fdf10b0b3f431f65e2da5e1687&searchStr=windows%2C2012%2Cx64%2Copt%2Cbuild-win64-msvc%2Fopt%2C%28bmsvc%29&fromchange=520d47760632c300e540b63b70e5655d60e9a8f4

Comment 2

[André Bargull [:anba]]

Are MSCV-only regressions still important to fix now that we build and ship Clang-based executables by default? I'm asking because I currently have neither a local set-up present to build MSCV nor do I know how to get the list of the "static constructors"  (TBH I don't actually know how and when a thing gets classified as a "static constructor"...) which caused this regression. That means for me I'd need to bisect the regression by investigating which of the 256 commits in https://github.com/unicode-org/icu/compare/release-62-1...maint/maint-63 is/are relevant, build ICU - including its data file - for each bisection step locally to prepare a patch file, which I then can upload to the Try-server for further processing by Perfherder.

Comment 3

[Ionuț Goldan [:igoldan]]

(In reply to André Bargull [:anba] from comment #2)
> Are MSCV-only regressions still important to fix now that we build and ship
> Clang-based executables by default?

:glandium should we disable perf monitoring for the MSVC builds?

Comment 4

[Nathan Froyd [:froydnj]]

The push in comment 0 is a) a backout and b) says that the offending changeset broke MSVC builds.  Is the changeset linked there the wrong changeset?

Comment 5

[Ionuț Goldan [:igoldan]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> The push in comment 0 is a) a backout and b) says that the offending
> changeset broke MSVC builds.  Is the changeset linked there the wrong
> changeset?

Oh, sorry about that. Indeed, that changeset is wrong; it was autogenerated and I forgot to check it.
It should have been https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=30dec3c6ac99c13c5a887441e17efa8a821009a6&tochange=b31fa37d257e44f611fc2e778aad242f97c2f1ec

Comment 6

[Nathan Froyd [:froydnj]]

Ah, thank you!

I think since this is an update of a third-party library that we don't generally patch, I think we should just close this as WORKSFORME, especially because the issue is not with currently-shipping products.

Comment 7

[André Bargull [:anba]]

Phew, lucky me! :-)