Skip to content
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

[DO NOT MERGE] Enable parallel compiler by default #79706

Closed
wants to merge 3 commits into from

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Dec 4, 2020

This is a rebase of #75651.

#78201 may have reduced the overhead of parallel rustc.

Aaron1011 and others added 2 commits December 4, 2020 19:53
It's been over a year since we last tried this - let's see what the
performance looks like now.
@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 4, 2020
@bjorn3
Copy link
Member Author

bjorn3 commented Dec 4, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Dec 4, 2020

⌛ Trying commit 6053526 with merge df18da5db6c9c66898610a29a2401e762c860f0a...

@bjorn3

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Dec 4, 2020

failures:
    [ui] ui/privacy/privacy2.rs
    [ui] ui/privacy/privacy3.rs
    [ui] ui/type_length_limit.rs

There are a few cases where the exact error changes.

@bors
Copy link
Contributor

bors commented Dec 4, 2020

☀️ Try build successful - checks-actions
Build commit: df18da5db6c9c66898610a29a2401e762c860f0a (df18da5db6c9c66898610a29a2401e762c860f0a)

@rust-timer
Copy link
Collaborator

Queued df18da5db6c9c66898610a29a2401e762c860f0a with parent 2218520, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (df18da5db6c9c66898610a29a2401e762c860f0a): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

@est31
Copy link
Member

est31 commented Dec 4, 2020

In the unit test benchmarks, there are sometimes really heavy regressions, but the wall clock time improvement for bigger crates is just beautiful:

Screenshot_20201204_221545


Screenshot_20201204_221333

@bjorn3
Copy link
Member Author

bjorn3 commented Dec 4, 2020

Instruction count difference seems to have gotten a bit worse since the last perf test, however wall time difference seems to have gotten a bit more positive than it already was.

@bjorn3
Copy link
Member Author

bjorn3 commented Dec 4, 2020

Going to do another perf run with only a single thread to see if the overhead in this case has been reduced.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Dec 4, 2020

⌛ Trying commit dec8166 with merge 7cd4a2ec5195e5df4cb2f7292aa53fa3d870cd9f...

@Mark-Simulacrum
Copy link
Member

The bootstrap times are actually really interesting, because perf runs that build under -j1 - which should mean that we get zero parallelism opportunity. I guess that means that either our -j1 isn't quite working or something else interesting is going on (e.g., we get better scheduling under parallel mode). Either way I'm not sure I'd trust these numbers given that...

Overall though other than that confusing element these do indeed look pretty good. We can't trust the instruction counts I think pretty much at all for this kind of assessment.

@bors
Copy link
Contributor

bors commented Dec 4, 2020

☀️ Try build successful - checks-actions
Build commit: 7cd4a2ec5195e5df4cb2f7292aa53fa3d870cd9f (7cd4a2ec5195e5df4cb2f7292aa53fa3d870cd9f)

@rust-timer
Copy link
Collaborator

Queued 7cd4a2ec5195e5df4cb2f7292aa53fa3d870cd9f with parent 2218520, future comparison URL.

@jyn514 jyn514 added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-compiler-parallel Working group: Parallelizing the compiler S-waiting-on-perf Status: Waiting on a perf run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2020
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (7cd4a2ec5195e5df4cb2f7292aa53fa3d870cd9f): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 5, 2020
@est31
Copy link
Member

est31 commented Dec 5, 2020

Now it's way less red in the instruction counts (single digit regressions), but there are regressions for the bootstrap timings.

@camelid camelid added the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Jan 15, 2021
@bjorn3 bjorn3 closed this Jan 23, 2021
@bjorn3 bjorn3 deleted the benchmark_parallel_rustc branch January 23, 2021 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-compiletime Issue: Problems and improvements with respect to compile times. S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-compiler-parallel Working group: Parallelizing the compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.