4 thread parallelism by default #67362
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Heh looks like one of our tests may be a little different in parallel mode! In any case r=me for tomorrow with tests passing |
ehuss
reviewed
Dec 16, 2019
src/librustc_session/config.rs
Outdated
| @@ -1362,7 +1362,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, | |||
| // a sequential compiler for now. This'll likely be adjusted | |||
There was a problem hiding this comment.
Maybe update the comment since it still mentions the default is 1?
This avoids the problems of high thread counts (i.e., contention in the kernel on the jobserver pipe due to thundering herd of readers) while stil giving rustc some parallelism to work with.
Mark-Simulacrum
force-pushed
the
par-4-default
branch
from
bdad80d to
6475be0
Compare
|
I finally understand why I've consistently seen that test fail for the last couple beta backports locally and just not understood how that can possibly be the case. Hopefully patched the test but I'll check again before I go to sleep... let's hope it deterministically fails, at least -- if not, I'll just delete it for now. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This also removes the unused NO_PARALLEL_COMPILER flag; if we want that functionality we can readd it but this makes sure we really are parallel everywhere. This also patches a test that has differing output in the parallel case (hopefully deterministically so!).
Mark-Simulacrum
force-pushed
the
par-4-default
branch
from
6475be0 to
47bb760
Compare
|
Okay, CI is passing here. It is past the UTC deadline for nightlies, and is ~21 hours till the next nightly is scheduled to be 'forked' off. So, @bors r=alexcrichton This hopefully ensures that we can get it in before the next nightly drops (and while we're at it, get some testing inside Rust's CI as well). |
|
📌 Commit 47bb760 has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened |
bors
added
S-waiting-on-bors
|
And since we want this urgently @bors p=100 |
|
⌛ Testing commit 47bb760 with merge eb33dd7e8c37024152db49ab4aa429e8b4a0d048... |
|
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
Chocolatey networking issues... |
|
@bors retry |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 47bb760 with merge 8753fb06fd9874a1eed1f6b0988e7b24ac5107e5... |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
Hm, so this failure might be spurious (i.e., timing dependent). @alexcrichton -- perhaps you've seen this before? Specifically, it looks like this test expects the build script to build first, but (maybe) with a parallel compiler we're not seeing that happen. I suspect that might be because we're capturing all the jobserver tokens pretty quickly so the build script rustc might not be able to start more than one codegen thread (i.e., the main thread) and as such is waiting on the bar crate to finish. I've kicked off a build locally of this branch with src/tools/cargo as the test target to hopefully dig in a bit deeper and am going to cede CI time to another PR for now. |
|
@bors retry If the other PR (beta promotion) fails let's make sure this gets testing right away since it might just pass. |
bors
added
S-waiting-on-bors
|
Yes that looks like a racy test in Cargo, we likely need to fix that |
|
I'm a bit surprised, because it uses Is it possible that the job server is getting confused with |
|
Yes, it's a known bug (that we should probably file an issue for) that rustc in parallel mode currently releases its implicit token; we have a fix planned but haven't yet gotten around to implementing it yet. In discussion with @alexcrichton we concluded that it probably makes sense to just disable Cargo's tests since those are likely inherently racy with this bug for now (we can revert this once we get a nightly out). @bors r=alexcrichton |
|
📌 Commit 8858851df08c3cbf657c9c2998af47e2a743d4c2 has been approved by |
|
⌛ Testing commit 8858851df08c3cbf657c9c2998af47e2a743d4c2 with merge 1805a917b2fbdfd02a156aa93d63ef652ce4487c... |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
These depend on rustc being bug-free and it looks like that's not currently entirely the case (e.g., we know of at least one bug that introduces nondeterminism).
Mark-Simulacrum
force-pushed
the
par-4-default
branch
from
8858851 to
5d4e59b
Compare
|
@bors r=alexcrichton |
|
📌 Commit 5d4e59b has been approved by |
bors
added a commit
that referenced
this pull request
Dec 17, 2019
4 thread parallelism by default The Session default here is super unusual but seems to both compile and do what we expect as best as I can tell.
|
☀️ Test successful - checks-azure |
This was referenced Dec 17, 2019
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
Dec 17, 2019
…, r=alexcrichton" This reverts commit 3ed3b8b, reversing changes made to 99b8953. We will reland a similar patch at a future date but for now we should get a nightly released in a few hours with the parallel patch, so this should be reverted to make sure that the next nightly is not parallel-enabled.
vivekvpandya
pushed a commit
to vivekvpandya/rust
that referenced
this pull request
Dec 18, 2019
…, r=alexcrichton" This reverts commit 3ed3b8b, reversing changes made to 99b8953. We will reland a similar patch at a future date but for now we should get a nightly released in a few hours with the parallel patch, so this should be reverted to make sure that the next nightly is not parallel-enabled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Session default here is super unusual but seems to both compile and do what we expect as best as I can tell.