New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc: Sort CGUs before merging #46918

Merged
merged 1 commit into from Dec 22, 2017

Conversation

Projects
None yet
6 participants
@alexcrichton
Member

alexcrichton commented Dec 21, 2017

This commit fixes some nondeterminism in compilation when using multiple codegen
units. The algorithm for splitting codegen units currently takes the
otherwise-would-be-for-incremental partitioning and then continuously merges the
two smallest codegen units until the desired number of codegen units are
reached.

We want to be sure to merge the same codegen units each time a compilation is
run but there's some subtle reorderings amongst all the items which was causing
this step to be slightly buggy. Notably this step involves sorting codegen units
by size, but if two codegen units had the same size they would appear in
different locations in the list each time.

This commit fixes this issue by sorting codegen units by name before doing the
loop to merge the two smallest. This means that we've got a deterministic
order going in and since we're using a stable sort this should mean that we're
always now getting a deterministic merging of codegen units.

Closes #46846

rustc: Sort CGUs before merging
This commit fixes some nondeterminism in compilation when using multiple codegen
units. The algorithm for splitting codegen units currently takes the
otherwise-would-be-for-incremental partitioning and then continuously merges the
two smallest codegen units until the desired number of codegen units are
reached.

We want to be sure to merge the same codegen units each time a compilation is
run but there's some subtle reorderings amongst all the items which was causing
this step to be slightly buggy. Notably this step involves sorting codegen units
by size, but if two codegen units had the same size they would appear in
different locations in the list each time.

This commit fixes this issue by sorting codegen units by name before doing the
loop to merge the two smallest. This means that we've got a deterministic
order going in and since we're using a stable sort this should mean that we're
always now getting a deterministic merging of codegen units.

Closes #46846
@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Dec 21, 2017

Collaborator

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

Collaborator

rust-highfive commented Dec 21, 2017

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Dec 21, 2017

@michaelwoerister

This comment has been minimized.

Show comment
Hide comment
@michaelwoerister
Contributor

michaelwoerister commented Dec 21, 2017

@bors r+

Thanks, @alexcrichton!

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Dec 21, 2017

Contributor

📌 Commit 427e630 has been approved by michaelwoerister

Contributor

bors commented Dec 21, 2017

📌 Commit 427e630 has been approved by michaelwoerister

kennytm added a commit to kennytm/rust that referenced this pull request Dec 21, 2017

Rollup merge of rust-lang#46918 - alexcrichton:fix-ordering, r=michae…
…lwoerister

rustc: Sort CGUs before merging

This commit fixes some nondeterminism in compilation when using multiple codegen
units. The algorithm for splitting codegen units currently takes the
otherwise-would-be-for-incremental partitioning and then continuously merges the
two smallest codegen units until the desired number of codegen units are
reached.

We want to be sure to merge the same codegen units each time a compilation is
run but there's some subtle reorderings amongst all the items which was causing
this step to be slightly buggy. Notably this step involves sorting codegen units
by size, but if two codegen units had the same size they would appear in
different locations in the list each time.

This commit fixes this issue by sorting codegen units by name before doing the
loop to merge the two smallest. This means that we've got a deterministic
order going in and since we're using a stable sort this should mean that we're
always now getting a deterministic merging of codegen units.

Closes rust-lang#46846

bors added a commit that referenced this pull request Dec 21, 2017

Auto merge of #46922 - kennytm:rollup, r=kennytm
Rollup of 15 pull requests

- Successful merges: #46636, #46780, #46784, #46809, #46814, #46820, #46839, #46847, #46858, #46878, #46884, #46890, #46898, #46916, #46918
- Failed merges:

kennytm added a commit to kennytm/rust that referenced this pull request Dec 21, 2017

Rollup merge of rust-lang#46918 - alexcrichton:fix-ordering, r=michae…
…lwoerister

rustc: Sort CGUs before merging

This commit fixes some nondeterminism in compilation when using multiple codegen
units. The algorithm for splitting codegen units currently takes the
otherwise-would-be-for-incremental partitioning and then continuously merges the
two smallest codegen units until the desired number of codegen units are
reached.

We want to be sure to merge the same codegen units each time a compilation is
run but there's some subtle reorderings amongst all the items which was causing
this step to be slightly buggy. Notably this step involves sorting codegen units
by size, but if two codegen units had the same size they would appear in
different locations in the list each time.

This commit fixes this issue by sorting codegen units by name before doing the
loop to merge the two smallest. This means that we've got a deterministic
order going in and since we're using a stable sort this should mean that we're
always now getting a deterministic merging of codegen units.

Closes rust-lang#46846

bors added a commit that referenced this pull request Dec 21, 2017

Auto merge of #46922 - kennytm:rollup, r=kennytm
Rollup of 14 pull requests

- Successful merges: #46636, #46780, #46784, #46809, #46814, #46820, #46839, #46847, #46858, #46878, #46884, #46890, #46898, #46918
- Failed merges:

@bors bors merged commit 427e630 into rust-lang:master Dec 22, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexcrichton alexcrichton deleted the alexcrichton:fix-ordering branch Jan 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment