We have detected a build metrics regression from push: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=8ee92682ad1f3b67c110742bc910cf1af03ac48b As author of one of the patches included in that push, we need your help to address this regression. Regressions: 5% compiler_metrics num_static_constructors linux64 asan 2,339.50 -> 2,459.83 5% build times windows2012-64 debug static-analysis taskcluster-c4.4xlarge2,203.69 -> 2,316.16 5% build times windows2012-32 debug static-analysis taskcluster-c4.4xlarge1,978.95 -> 2,069.44 1% compiler_metrics num_static_constructors linux64 debug 234.54 -> 236.83 1% compiler_metrics num_static_constructors linux64 opt 122.33 -> 123.17 0% installer size windows2012-64 pgo 61,833,842.83 -> 62,107,776.00 0% installer size windows2012-32 pgo 57,778,726.04 -> 57,985,596.83 0% installer size windows2012-64 opt rusttests 60,982,759.62 -> 61,196,638.17 0% installer size windows2012-64 opt 60,983,036.96 -> 61,196,310.00 0% compiler_metrics num_static_constructors linux64 pgo 125.00 -> 125.42 0% installer size windows2012-64 opt static-analysis 61,264,843.88 -> 61,438,897.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=11900 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

:jgilbert Bug 1440849 caused multiple types of regressions. Installer size and build times are prioritary, in this order. Please address these.

ANGLE is a big external dependency, so it's possible there might not be too much we can do about it. That said, this is a pretty large size regression given that we're only upgrading. I think it would be worthwhile using code coverage metrics to determine if there are any large chunks of ANGLE we're not using that we could stub out. Also worth figuring out what those static ctors are and ideally fixing them.

Does 'static ctors' include locally-defined statics, or is it just global statics? We expect a sizable increase in local statics from at least some of the changes I know about, but it shouldn't increase the global count. Here's a comment one source of binary size increase: https://chromium-review.googlesource.com/c/angle/angle/+/786317#message-ed6097b012b23a8b46286d23bb58ecc0f10ecf13 > libGLESv2.dll MSVC Win32/Release: > 5282816B -> 5307904B = +25088B > > libGLESv2.dll Clang x6/Release: > 7322112B -> 7343104B = +20992B > > on Linux: > > libGLESv2.so Clang x64/Release: > 6305040B -> 6356568B = +51528B From this change alone, we're looking at +50KB on Windows (+25KB from both libGLESv2.dll and libANGLE in xul), +50KB on non-windows. (just libANGLE in xul) We had to make modifications to convert their constexpr code into templated-local-statics, so I wouldn't be surprised if this change at least doubled those increases. It would be interesting to get numbers with constexpr re-enabled. (It's disabled for now, because we need c++14 constexpr, but we still support compilers that don't offer c++14 constexpr) This gets us to roughly the right order of magnitude of increase as what we're seeing. (+274KB for Win64 PGO) Build time increase for static analysis builds is probably commensurate with adding so many template instantiations, as well as just general increase from library changes.

(In reply to Jeff Gilbert [:jgilbert] from comment #3) > Does 'static ctors' include locally-defined statics, or is it just global > statics? We expect a sizable increase in local statics from at least some of > the changes I know about, but it shouldn't increase the global count. It is just global statics. It's possible that the way ANGLE got built shuffled some global ctors around so that global ctors that were grouped in the same unified file are no longer grouped together, thus "creating" new global ctors.

(In reply to Nathan Froyd [:froydnj] from comment #4) > (In reply to Jeff Gilbert [:jgilbert] from comment #3) > > Does 'static ctors' include locally-defined statics, or is it just global > > statics? We expect a sizable increase in local statics from at least some of > > the changes I know about, but it shouldn't increase the global count. > > It is just global statics. It's possible that the way ANGLE got built > shuffled some global ctors around so that global ctors that were grouped in > the same unified file are no longer grouped together, thus "creating" new > global ctors. Oh, yeah, that's another change that probably did it. I was having trouble getting UNIFIED_SOURCES to build it well as-is, so I went full-SOURCES for ANGLE, which, while simpler, could definitely hit those numbers.

Is this bug actionable then?

Switching back to UNIFIED_SOURCES and switching to relaxed constexpr after c++14 is required should claw this back, for the most part. These should be coming in 61.

Also of note: this regressed our .text size by ~76k and .data size by ~17k on Linux64.

I can take a stab at this.

(In reply to Jeff Gilbert [:jgilbert] from comment #3) > We had to make modifications to convert their constexpr code into > templated-local-statics, so I wouldn't be surprised if this change at least > doubled those increases. It would be interesting to get numbers with > constexpr re-enabled. (It's disabled for now, because we need c++14 > constexpr, but we still support compilers that don't offer c++14 constexpr) > This gets us to roughly the right order of magnitude of increase as what > we're seeing. (+274KB for Win64 PGO) > Can we use constexpr in compilers that support it? All of our automation builds for tier 1 platforms already should...

We use constexpr now. (bug 1455782)

The trivial s/SOURCES/UNIFIED_SOURCES/ doesn't work: https://treeherder.mozilla.org/#/jobs?repo=try&revision=63d48ec4d5711779ef006c9ba390da5cf953f05f&selectedJob=181998331

non-unified: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e2211bf15edd14107be37cbbf66f97854241b70&selectedJob=182177332 unified: https://treeherder.mozilla.org/#/jobs?repo=try&revision=12e64ebae0c24001cd27256ac20223034f7606b1&selectedJob=182176179 On Linux, seems to drop XUL code size by 40k, installer size by 250k. On Mac, seems to /increase/ installer size by 60k. On Win, seems to /increase/ installer size by 13k. Nothing else seems to change. I didn't ask for PGO builds, should I bother? Is there anything else I should look at here?

(In reply to Jeff Gilbert [:jgilbert] from comment #13) > non-unified: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=9e2211bf15edd14107be37cbbf66f97854241b70&selectedJob=1 > 82177332 > > unified: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=12e64ebae0c24001cd27256ac20223034f7606b1&selectedJob=1 > 82176179 > > On Linux, seems to drop XUL code size by 40k, installer size by 250k. > On Mac, seems to /increase/ installer size by 60k. > On Win, seems to /increase/ installer size by 13k. > > Nothing else seems to change. I didn't ask for PGO builds, should I bother? > Is there anything else I should look at here? I don't think PGO builds are needed. They seemed to mimic the OPT builds. Something else to check would be the 5% compiler_metrics num_static_constructors from Linux 64 asan builds.

:jmaher I'm not sure about the relevance of compiler_metrics num_staic_constructors regressions on asan builds. How important is it for us to resolve them? I know that we can ignore similar regressions from debug builds.

lets ask the owner, I would say it is not important.

Static constructors are not important for ASan builds. We should turn them off (preferably in another bug).

(In reply to Nathan Froyd [:froydnj] from comment #17) > Static constructors are not important for ASan builds. We should turn them > off (preferably in another bug). Should I file that bug or have you already done that?

Please file it. Thank you!

Jeff, what's the status on getting back the installer size regressions here?

I tried using UNIFIED_SOURCES, but that actually /increased/ install size on Mac and Windows, so we probably don't want that. Further investigation is P5 to me, but there's no clear path forward here, so I'm calling WONTFIX. Pulling parts out of ANGLE should be an approach of last resort. I would rather look into obviating our need for ANGLE entirely, and would rather pull code out of elsewhere in Gecko if it's essential to claw back these kilobytes.