Update builders to rust 1.28

RESOLVED FIXED in Firefox 63

Status

enhancement
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: chmanchester, Assigned: glandium)

Tracking

(Blocks 1 bug)

Trunk
mozilla63
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

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
Depends on: 1447137
Blocks: 1450077
Blocks: 1450078
Comment hidden (mozreview-request)
Attachment #8963825 - Flags: review?(core-build-config-reviews) → review?(nalexander)

Comment 2

a year ago
mozreview-review
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+

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/37c026e3a6a5
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
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
Assignee

Comment 8

a year ago
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.
Assignee

Comment 10

11 months ago
FWIW, 1.28.0-beta.1 is beta-2018-06-22 for --channel.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8987770 - Flags: review?(core-build-config-reviews) → review?(nfroyd)

Comment 15

11 months ago
mozreview-review
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 16

11 months ago
mozreview-review
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+
Reporter

Comment 17

11 months ago
mozreview-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.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 20

11 months ago
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)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Reporter

Updated

11 months ago
Depends on: 1471386
Reporter

Updated

11 months ago
Assignee: cmanchester → mh+mozilla
See Also: → 1471443
Assignee

Updated

11 months ago
Depends on: 1471497
Assignee

Updated

11 months ago
Attachment #8963825 - Attachment is obsolete: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 27

11 months ago
mozreview-review
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
Assignee

Comment 34

11 months ago
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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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

Comment 44

11 months ago
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.
Reporter

Comment 45

11 months ago
Merged to central in comment 38.
Status: REOPENED → RESOLVED
Last Resolved: a year ago11 months 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.
Reporter

Comment 47

11 months ago
(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 → ---
Reporter

Updated

11 months ago
Blocks: 1472857
Assignee

Comment 48

11 months ago
Flags: needinfo?(mh+mozilla)
Assignee

Comment 49

11 months ago
Assignee

Comment 50

11 months ago
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.
Assignee

Comment 51

11 months ago
I'll note that gkrust waiting for gkrust_gtest to finish is weird too.
Blocks: 1473069
Reporter

Updated

10 months ago
Depends on: 1480605
Assignee

Comment 52

10 months ago
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
Last Resolved: 11 months ago10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.