1421100 - Update win32 builders to rust 1.23.0-beta.2

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ralph Giles (:rillian)]

Rust 1.22 fails to compile gkrust for win32 with -Clto. We think this is just address space exhaustion (bug 1417268) and Rust 1.23 doesn't show the issue because of more efficient code representation passed to llvm.

Using 64-bit Rust to compile for win32 is blocked on bug 1401647. In the meantime, bump win32 builders to 1.23.0-beta.1 so we're not limited to 1.21 as a minimum version. Rust 1.23 is expected to be in stable release before the current Firefox Nightly goes to beta in January.

Comment 1

[Ralph Giles (:rillian)]

Attachment: Bug 1421100 - Update win32 builders to rust 1.23.0-beta.2.

Work around address space exhaustion with 32-bit Rust
on Windows by building with the current beta, which
does not show the problem, likely because of more
efficient llvm ir generation.

See bug 1417268 for discussion.

Review commit: https://reviewboard.mozilla.org/r/203296/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203296/

Comment 2

[Mike Hommey [:glandium]]

I don't think we should use rustc betas.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8932279 [details]
Bug 1421100 - Update win32 builders to rust 1.23.0-beta.2.

https://reviewboard.mozilla.org/r/203296/#review208910

::: taskcluster/ci/toolchain/windows.yml:149
(Diff revision 1)
> -win32-rust-1.21:
> +win32-rust-1.23.0-beta.1:
>      description: "rust repack"
>      treeherder:
>          kind: build
>          platform: toolchains/opt
> -        symbol: TW32(rust)
> +        symbol: TW32(rust-beta)

Doesn't seem particularly useful to change the Treeherder symbol here.

Comment 4

[Ralph Giles (:rillian)]

Comment on attachment 8932279 [details]
Bug 1421100 - Update win32 builders to rust 1.23.0-beta.2.

https://reviewboard.mozilla.org/r/203296/#review208910

> Doesn't seem particularly useful to change the Treeherder symbol here.

I thought it was more clear, and means I don't have to change it to test a stable release in parallel. I'll take it out.

Comment 5

[Ralph Giles (:rillian)]

Green on try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=45c4030f177696e6e438671f3c77a1667aa8a876

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2f814fa49aa
Update win32 builders to rust 1.23.0-beta.1. r=ted

Comment 7

[Ralph Giles (:rillian)]

(In reply to Mike Hommey [:glandium] from comment #2)

> I don't think we should use rustc betas.

I know, but that's not the policy Mozilla decided on. :/

Comment 8

[Tiberius Oros[:tiberius_oros]]

Backed out for failing Windows x32 web platform tests /encoding/api-replacement-encodings.html
Backout https://hg.mozilla.org/integration/autoland/rev/488e18d45352a9845ce509d3c1a0b22ab642a6ce

Push causing the failures https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c2f814fa49aac64334358279b3dc186944a967d4

Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=148237887&repo=autoland&lineNumber=3463

INFO - 
3463
19:02:13     INFO - TEST-UNEXPECTED-FAIL | /encoding/api-replacement-encodings.html | Label for "replacement" should be rejected by API: csiso2022kr - assert_throws: function "function() { new TextDecoder(label); }" did not throw
3464
19:02:13     INFO - @http://web-platform.test:8000/encoding/api-replacement-encodings.html:14:17
3465
19:02:13     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1489:20
3466
19:02:13     INFO - test@http://web-platform.test:8000/resources/testharness.js:511:9
3467
19:02:13     INFO - @http://web-platform.test:8000/encoding/api-replacement-encodings.html:13:13
3468
19:02:13     INFO - @http://web-platform.test:8000/encoding/api-replacement-encodings.html:12:9
3469
19:02:13     INFO - @http://web-platform.test:8000/encoding/api-replacement-encodings.html:9:5
3470
19:02:13     INFO - @http://web-platform.test:8000/encoding/api-replacement-encodings.html:8:1

Comment 9

[Ralph Giles (:rillian)]

Reproduces upstreeam. I filed hsivonen/encoding_rs#26

Comment 11

[(not currently active) Ted Mielczarek]

(In reply to Mike Hommey [:glandium] from comment #2)
> I don't think we should use rustc betas.

We agreed that it was OK to *use* betas for our CI builds as long as we didn't *require* a non-release version, with the intention being that we would always wind up shipping release Firefox with a release Rust.

Comment 12

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #11)
> (In reply to Mike Hommey [:glandium] from comment #2)
> > I don't think we should use rustc betas.
> 
> We agreed that it was OK to *use* betas for our CI builds as long as we
> didn't *require* a non-release version, with the intention being that we
> would always wind up shipping release Firefox with a release Rust.

But the goal here is to allow to bump the requirement to 1.22, which we can't build with. So effectively, this would make the beta a requirement.

Comment 13

[Ralph Giles (:rillian)]

(In reply to Mike Hommey [:glandium] from comment #12)

> But the goal here is to allow to bump the requirement to 1.22, which we
> can't build with. So effectively, this would make the beta a requirement.

The build failure with 1.22 only affects automation, so there should be no requirement on downstream builders to use beta rust.

Comment 14

[Mike Hommey [:glandium]]

There is no reason for it not to affect local builds of 32-bits Firefox.

Comment 15

[Ralph Giles (:rillian)]

1.23.0-beta.2 should be out soon with a patch which addresses the encoding_rs test regression for me.

Comment 16

[Ralph Giles (:rillian)]

Comment on attachment 8932279 [details]
Bug 1421100 - Update win32 builders to rust 1.23.0-beta.2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/203296/diff/1-2/

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5986b2cc92b
Update win32 builders to rust 1.23.0-beta.2. r=ted

Comment 18

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/e5986b2cc92b

Comment 19

[Mike Hommey [:glandium]]

So thanks to this change we're now in a funny position, where the rust-1.23.0-beta.2 job is downloading 1.24.0-beta.1, and that version breaks building slog.

Comment 20

[Mike Hommey [:glandium]]

oh, and 1.23.0 is not released yet, so we can't even workaround by forcing back to version 1.23.0, because it doesn't exist.

Comment 21

[Mike Hommey [:glandium]]

and there doesn't seem to be a way to download specific beta versions...

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5afa32a16a2f
Followup for bug 1421100: deterministically download rustc 1.23.0-beta.2. r=me, a=bustage on a CLOSED TREE

Comment 23

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/5afa32a16a2f

Comment 24

[Ionuț Goldan [:igoldan]]

These changes improved performance on two of our tests:

== Change summary for alert #11120 (as of Wed, 10 Jan 2018 05:34:27 GMT) ==

Improvements:

  6%  Stylo Servo_DeclarationBlock_SetPropertyById_Bench windows7-32 opt      519,893.42 -> 488,779.50
  6%  Stylo Servo_DeclarationBlock_SetPropertyById_WithInitialSpace_Bench windows7-32 opt 521,590.29 -> 491,489.42

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