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 #75651

Closed
wants to merge 2 commits into from

Conversation

Aaron1011
Copy link
Member

It's been over a year since we last tried this - let's see what the
performance looks like now.

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? @Mark-Simulacrum

(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 Aug 17, 2020
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Aug 17, 2020

⌛ Trying commit a90aa76 with merge f38982d9a59604e6e97ac1fe0c0267eefddb05d5...

@Mark-Simulacrum
Copy link
Member

Note that there are known scalability flaws, and that this is insufficient by itself - you need to flip https://github.com/rust-lang/rust/blob/master/src/librustc_session/options.rs#L1033 to a more reasonable default (iirc, it's annoying to get num CPUs there, so you might want to just put 6 or 12, which are the physical/virtual thread counts on perf.rlo's collector)

That said, it would be interesting to see the overhead of the worst case (locks enabled but useless) on modern rustc. Maybe best to do that after we've bumped parking lot and other crates to latest versions, though.

@bors
Copy link
Contributor

bors commented Aug 18, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: f38982d9a59604e6e97ac1fe0c0267eefddb05d5 (f38982d9a59604e6e97ac1fe0c0267eefddb05d5)

@rust-timer
Copy link
Collaborator

Queued f38982d9a59604e6e97ac1fe0c0267eefddb05d5 with parent 792c645, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (f38982d9a59604e6e97ac1fe0c0267eefddb05d5): 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

@mati865
Copy link
Contributor

mati865 commented Aug 18, 2020

Instructions look not that bad. Wall-time and task-clock regress noticeably.

@joshtriplett
Copy link
Member

@Aaron1011 Could you give this another run with a change to the default thread count, to use 12 (or num_cpus) by default?

@Mark-Simulacrum
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Aug 29, 2020

⌛ Trying commit 9a17037 with merge d41fa410d2663e58bbf1cbaf20508cf341ea76ab...

@bors
Copy link
Contributor

bors commented Aug 29, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: d41fa410d2663e58bbf1cbaf20508cf341ea76ab (d41fa410d2663e58bbf1cbaf20508cf341ea76ab)

@rust-timer
Copy link
Collaborator

Queued d41fa410d2663e58bbf1cbaf20508cf341ea76ab with parent 286a346, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (d41fa410d2663e58bbf1cbaf20508cf341ea76ab): 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

@mati865
Copy link
Contributor

mati865 commented Aug 29, 2020

Mix of massive wins and big regressions in wall-time.

@Mark-Simulacrum
Copy link
Member

Closing -- I think this is perhaps something to look at, but I don't currently have the bandwidth to do so and it looks like @Aaron1011 doesn't either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants