Closed Bug 1578254 Opened 5 years ago Closed 5 years ago

10.81% build times (windows2012-aarch64) regression on push c8660505bc7e65f20c5959fb4e940df17a1c3d9e (Tue August 27 2019)

Categories

(Core :: Networking, defect, P2)

All
Unspecified
defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: marauder, Assigned: glandium)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [necko-triaged])

Attachments

(1 file)

We have detected a build metrics regression from push:

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

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

Regressions:

11% build times windows2012-aarch64 opt aarch64-no-eme nightly taskcluster-c4.4xlarge 3,664.54 -> 4,060.69

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

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

Blocks: 1562138
Component: Performance → Networking
Flags: needinfo?(michal.novotny)
Product: Testing → Core
Hardware: Unspecified → All
Target Milestone: --- → mozilla70

Bug 1567616 adds a code that builds only on Linux. On windows it just changes prefs to static prefs.

Flags: needinfo?(michal.novotny)

Thanks for the info Michal.

Chris, Mike - in this alert pushlog :
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2bebda12a12a64915ad26887bdf94c503bc6103d&tochange=c8660505bc7e65f20c5959fb4e940df17a1c3d9e
there are some patches related to Firefox Build System that you created,
do you think that this regression might be related to those changes?
Thanks!

Flags: needinfo?(mh+mozilla)
Flags: needinfo?(cmanchester)

The graph points at bug 1575760, but that makes no sense. I mean, it's not supposed to have changed anything that would impact build times. Also, why only these builds?

Flags: needinfo?(mh+mozilla)
Blocks: 1578356
No longer blocks: 1562138

I'm pretty sure my patch didn't impact this build. But this is a significant increase and it's hard to tell where it's coming from.

Flags: needinfo?(cmanchester)

Add [necko-triaged] for removing this from out bug list.

Priority: -- → P2
Whiteboard: [necko-triaged]
Assignee: nobody → mh+mozilla
Regressed by: 1575760

One of the things bug 1567616 did was to change how .cargo/config is
preprocessed, from it happening during configure to it happening during
the build. And to make things happen properly, dependencies were added
on the rust targets to ensure the .cargo/config file is created before
they run.

Unfortunately, that changed the order in which Make would run all the
targets while recursing for the compile tier, when the file doesn't
exist first. So instead of starting the compile tier with rust targets,
it would start with most C++, then do rust... which we know to make
builds slower overall because of the need to wait for those rust builds
to finish which C++ has all been dealt with already, and lacking
parallelism during the rust build.

So instead of using the ideal dependencies, we force .cargo/config to be
generated during export (which it is not already because OBJDIR_PP_FILES
are currently dealt with during misc).

As the rust_targets variable is not used anymore, we remove its
creation from recursivemake.py.

And while here, we extend the if block in recurse.mk that excludes all
the top-level recursion dependencies when running from subdirectories.

Attachment #9091003 - Attachment description: Bug 1578254 - Trick Make into running rust target earlier again. → Bug 1578254 - Trick Make into running rust targets earlier again.
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/a4f0f505702b Trick Make into running rust targets earlier again. r=nalexander
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Seems like this would be good to get uplifted to Beta at some point.

Thank you Mike!
After your patch landed, perfherder created a new alert for improvements :

== Change summary for alert #22985 (as of Sat, 07 Sep 2019 19:09:27 GMT) ==

Improvements:

12% build times windows2012-aarch64 opt aarch64-no-eme nightly taskcluster-c4.4xlarge 4,183.02 -> 3,701.56

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22985

Flags: needinfo?(mh+mozilla)
No longer regressed by: 1575760
Regressed by: 1575760

Comment on attachment 9091003 [details]
Bug 1578254 - Trick Make into running rust targets earlier again.

Beta/Release Uplift Approval Request

  • User impact if declined: Improve build times
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • 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): Minor build system fixup for a regression from bug 1575760
  • String changes made/needed:
Attachment #9091003 - Flags: approval-mozilla-beta?

Comment on attachment 9091003 [details]
Bug 1578254 - Trick Make into running rust targets earlier again.

Fixes a regression causing builds to run slower. Approved for 70.0b8.

Attachment #9091003 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1592626
No longer blocks: 1592626
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: