Closed Bug 1447116 Opened 6 years ago Closed 6 years ago

Update builders to rust 1.28

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: chmanchester, Assigned: glandium)

References

Details

Attachments

(4 files, 1 obsolete file)

The 1.25 is scheduled for March 29, we should be ready to update the toolchains used in integration branch builds by then.
Assignee: nobody → cmanchester
Attachment #8963825 - Flags: review?(core-build-config-reviews) → review?(nalexander)
Comment on attachment 8963825 [details]
Bug 1447116 - Update rust builders to 1.25

https://reviewboard.mozilla.org/r/232698/#review238144

Technically, this looks fine.
Attachment #8963825 - Flags: review?(nalexander) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/37c026e3a6a5
Update rust builders to 1.25 r=nalexander
https://hg.mozilla.org/mozilla-central/rev/37c026e3a6a5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Depends on: 1451703
Depends on: 1456150
Per bug 1451703 and bug 1456150, can we please back this out from m-c before the soft freeze starts on Thursday?
Flags: needinfo?(cmanchester)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla61 → ---
Version: Version 3 → Trunk
For the reasons discussed in bug 1456150, rustc 1.28 is likely to be our next upgrade, on or around August 16th.
Summary: Update builders to rust 1.25 → Update builders to rust 1.28
We may want to jump ship early by using 1.28 when it is beta on nightly. As long as we keep the base toolchains job with the minimum version we support (which we could say is the corresponding stable, 1.27), we're good.
(In reply to Mike Hommey [:glandium] from comment #8)
> We may want to jump ship early by using 1.28 when it is beta on nightly. As
> long as we keep the base toolchains job with the minimum version we support
> (which we could say is the corresponding stable, 1.27), we're good.

That's a good idea. I'll get that going at some point after the merge.
FWIW, 1.28.0-beta.1 is beta-2018-06-22 for --channel.
Attachment #8987770 - Flags: review?(core-build-config-reviews) → review?(nfroyd)
Comment on attachment 8987770 [details]
Bug 1447116 - Require rust 1.26.

https://reviewboard.mozilla.org/r/253040/#review259662

r=me with the obvious below change.

::: commit-message-2bb75:3
(Diff revision 1)
> +We're well overdue for an upgrade of the rust compiler requirements.
> +Now that we're building with 1.28 (albeit a beta, due to be bumped when
> +it's released), we can bump the requirement away from 1.24 which is now
> +old. 1.27 is too new, though, so settle for the older 1.26.

If we're going to bump the base toolchain job, presumably we also want to bump the requirement in rust.configure:

https://searchfox.org/mozilla-central/source/build/moz.configure/rust.configure#70

?
Attachment #8987770 - Flags: review?(nfroyd) → review+
Attachment #8987762 - Flags: review?(core-build-config-reviews) → review?(nfroyd)
Comment on attachment 8987762 [details]
Bug 1447116 - Update builders to rust 1.28.

https://reviewboard.mozilla.org/r/253036/#review259666

Sure, we can do this.
Attachment #8987762 - Flags: review?(nfroyd) → review+
Comment on attachment 8987770 [details]
Bug 1447116 - Require rust 1.26.

https://reviewboard.mozilla.org/r/253040/#review259768

::: commit-message-2bb75:4
(Diff revision 1)
> +Now that we're building with 1.28 (albeit a beta, due to be bumped when
> +it's released), we can bump the requirement away from 1.24 which is now
> +old. 1.27 is too new, though, so settle for the older 1.26.

If we're going to make this change here it should be accompanied by a change to bootstrap.
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 497fc5c45f44239a54263639268361e5e521cfe2 -d 9f4d1a4296ba: rebasing 469906:497fc5c45f44 "Bug 1447116 - Update builders to rust 1.28. r=froydnj"
merging taskcluster/ci/toolchain/windows.yml
warning: conflicts while merging taskcluster/ci/toolchain/windows.yml! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Assignee: cmanchester → mh+mozilla
See Also: → 1471443
Depends on: 1471497
Attachment #8963825 - Attachment is obsolete: true
Comment on attachment 8987770 [details]
Bug 1447116 - Require rust 1.26.

https://reviewboard.mozilla.org/r/253040/#review259950

::: build/moz.configure/rust.configure:71
(Diff revisions 3 - 4)
>  
>          You can install rust by running './mach bootstrap'
>          or by directly running the installer from https://rustup.rs/
>          '''))
>      rustc_min_version = Version('1.26.0')
> -    cargo_min_version = Version('0.{}'.format(rustc_min_version.minor + 1))
> +    cargo_min_version = rustc_min_version

I learned today that as of 1.26 cargo has the same version as rustc, so changing that at the same time. That can't reland until bug 1471497 is figured out, though.
When this landed (comment 23), we noticed these build_metrics regressions:

== Change summary for alert #14048 (as of Tue, 26 Jun 2018 23:20:22 GMT) ==

Regressions:

 37%  build times windows2012-64 opt plain taskcluster-c4.4xlarge     1,969.34 -> 2,707.11
  0%  installer size osx-cross opt                                    65,616,489.92 -> 65,903,526.00
  0%  installer size linux32 pgo                                      63,218,017.50 -> 63,430,315.17
  0%  installer size linux64 pgo                                      62,524,675.50 -> 62,717,406.83

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=14048
It looks like *landing 1.28* made build times *substantially* better: it was the backout that caused the regression.

https://treeherder.mozilla.org/perf.html#/graphs?timerange=1209600&series=autoland,1682241,1 shows a graph. The original Perfherder alert showing the improvement is at https://treeherder.mozilla.org/perf.html#/alerts?id=14037.
It appears Linux also saw a major bump: https://treeherder.mozilla.org/perf.html#/graphs?timerange=604800&series=autoland,1682289,1,2&series=autoland,1682265,1,2&series=autoland,1691945,1,2&series=autoland,1682223,1,2&series=autoland,1682281,1,2&series=autoland,1697287,1,2

But this was backed out before a Perfherder alert could be generated. We should see the alerts when this sticks its landing.
> it was the backout that caused the regression

This is great to read! I was *very* worried for a while there :)
(In reply to Gregory Szorc [:gps] from comment #29)
> It looks like *landing 1.28* made build times *substantially* better: it was
> the backout that caused the regression.
> 
> https://treeherder.mozilla.org/perf.html#/
> graphs?timerange=1209600&series=autoland,1682241,1 shows a graph. The
> original Perfherder alert showing the improvement is at
> https://treeherder.mozilla.org/perf.html#/alerts?id=14037.

Oh, I apologize for making this confusion. I looked at the wrong alert. :gps thank you for correcting me!
The correct results are these:

== Change summary for alert #14037 (as of Tue, 26 Jun 2018 19:02:03 GMT) ==

Improvements:

 26%  build times windows2012-64 opt plain taskcluster-c4.4xlarge     2,664.49 -> 1,961.31
  4%  build times linux64 opt plain taskcluster-c4.4xlarge            2,052.91 -> 1,968.52

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=14037
1.28 beta 6 has the fix for bug 1471497. I'll update the patches and land that, and keep this bug open to update to final 1.28 when it's released at the beginning of August.
Keywords: leave-open
I couldn't help but notice that the compile speedups seem to mostly have an impact on opt builds. It appears opt builds in CI are now almost as fast as debug builds.

We had previously changed the Rust optimization level on local builds to a lower level to speed up builds. This was definitely a compromise and a handful of people complained about this because it meant local builds no longer had reasonable performance/behavior characteristics by default. With optimized builds apparently getting a hefty speedup, should we revisit this decision and make local builds use higher optimization levels again [if Rust is sufficiently modern]?
Flags: needinfo?(mh+mozilla)
I guess we may need a separate task to enforce that code gets pushed still builds with 1.26, otherwise people may start breaking the 1.26 requirement by using new features from 1.27 and 1.28.
(In reply to Xidorn Quan [:xidorn] UTC+10 (less responsive until July 7) from comment #40)
> I guess we may need a separate task to enforce that code gets pushed still
> builds with 1.26, otherwise people may start breaking the 1.26 requirement
> by using new features from 1.27 and 1.28.

They're called the linux64-base-toolchain tasks:

https://searchfox.org/mozilla-central/source/taskcluster/ci/build/linux.yml#242
Oh, okay, thanks for the pointer. Maybe it's worth having something similar for Windows and macOS as well, but maybe there aren't too many platform-specific code in Rust so it's probably not too bad. It may still make sense to have that for C++/ObjC code though.
Talos also noticed perf improvements:

== Change summary for alert #14131 (as of Mon, 02 Jul 2018 04:00:17 GMT) ==

Improvements:

  3%  displaylist_mutate windows10-64-qr opt e10s stylo     4,861.50 -> 4,731.66
  2%  displaylist_mutate linux64-qr opt e10s stylo          5,323.16 -> 5,206.26

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=14131
Also note we saw a ~580K reduction in libxul section sizes with this push, primarily in .text and .rodata but also a 26K reduction in .data.rel.ro.
Merged to central in comment 38.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
glandium wanted to keep this open per comment 34. As long as we have an open bug blocking release to track upgrading to non-beta 1.28, I don't care whether we reopen this or file a new bug. But I think sheriffs prefer new bugs over bugs with patches landing over the span of weeks.
(In reply to Gregory Szorc [:gps] from comment #46)
> glandium wanted to keep this open per comment 34. As long as we have an open
> bug blocking release to track upgrading to non-beta 1.28, I don't care
> whether we reopen this or file a new bug. But I think sheriffs prefer new
> bugs over bugs with patches landing over the span of weeks.

Yes, missed that, my mistake.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: needinfo?(mh+mozilla)
If you look at these graphs you'll see how crazy long both gkrust and gkrust_gtest were taking to build with 1.24, compared to 1.28, and that's most probably LTO. So that can't really tell much about what optimization levels changes would do to local builds.
I'll note that gkrust waiting for gkrust_gtest to finish is weird too.
Erf, I had kept this open for the update to the final release, but filed a separate bug for that, so let's close this.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.