1585558 - 93.28 - 264.17% build times (android-4-0-armv7-api16, linux32-shippable, linux64-shippable) regression on push 16d04cad3cf99f30a8abb7892a280847d32c1822 (Sat September 28 2019)

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect, P3)

Description

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

We have detected a build metrics regression from the push below. We don't have the tools for bisecting this kind of regression so I am asking if it makes sense for any of you. Also, I don't have access to bug 1583368 so if anyone could help pinging relevant people I'd be grateful.

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=195277db19c51031b4ba4b01bbd6e36fc1b18104&tochange=16d04cad3cf99f30a8abb7892a280847d32c1822

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

Regressions:

264% build times linux64-shippable opt instrumented taskcluster-c5d.4xlarge 401.73 -> 1,463.00
236% build times linux64-shippable opt instrumented taskcluster-c5.4xlarge 447.84 -> 1,506.67
192% build times linux64-shippable opt instrumented taskcluster-m5.4xlarge 539.71 -> 1,576.41
190% build times linux32-shippable opt instrumented taskcluster-c5d.4xlarge 491.77 -> 1,426.21
165% build times linux32-shippable opt instrumented taskcluster-c5.4xlarge 524.95 -> 1,391.04
144% build times linux32-shippable opt instrumented taskcluster-m5.4xlarge 599.78 -> 1,465.01
124% build times android-4-0-armv7-api16 pgo instrumented taskcluster-c5.4xlarge 597.36 -> 1,339.64
111% build times android-4-0-armv7-api16 pgo instrumented taskcluster-m5.4xlarge 725.75 -> 1,533.82
93% build times android-4-0-armv7-api16 pgo instrumented taskcluster-c5d.4xlarge 738.73 -> 1,427.83

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

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

[Ryan Hunt [:rhunt]]

Wrong Ryan :)

Comment 2

[J.C. Jones [:jcj] (he/they)]

I'm guessing it's bug 1583368 and thus maybe an expected regression, but passing my needinfo on.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Yes, this an expected regression from that change. That said, I'm pretty shocked it was that large of one since the change was only supposed to affect one phase of the PGO build.

Comment 4

[Ricky Stewart]

Yes -- a performance regression for these (shippable) builds would be expected.

Comment 5

[Mike Hommey [:glandium]]

Just shows how much sccache makes builds faster.

Comment 6

[Daniel Holbert [:dholbert]]

Is there anything to done here, or should we close this as INVALID or WONTFIX given comment 3 and comment 4?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Sounds like a wontfix to me.

Comment 8

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

Giving the magnitude of this, as a sheriff is my job to make sure I understand the implications. So, Ryan or Ricky, are you kind enough to explain why a regression of 1,000KB (the minimum threshold is 10 times smaller - 100KB) is resolved as WONTFIX? Thanks!

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Hi Alexandru, I'd be happy to explain. To be clear, this was a regression in build time, not in any size or perf metrics affecting the browser we ship. More specifically, the regressing change specifically disabled a build performance optimization for the shippable builds for a bit of security hardening. I've CCed you to that bug for a bit more background information. Anyway, we specifically disabled something which speeds up builds and we saw a regression in build times as one would expect from such a change. That was a known trade-off, hence the wontfix here.

Feel free to ping if you have any follow-up questions!

Comment 10

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

Thank you, Ryan.