Refactor passes and pass execution to be more parallel #58679
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 |
|
☔ The latest upstream changes (presumably #57760) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Rebased #58250 away. |
|
@bors try |
|
⌛ Trying commit c9f00f12b95802fe6a2ffee0af90ea896fb4c135 with merge 48f80d17254f15148d650224c66260acd4e1de95... |
|
Oh wait, that won't show up on perf.rlo... |
|
@rust-lang/infra is there a way to cancel a try-build? |
This comment has been minimized.
This comment has been minimized.
bors
added
S-waiting-on-author
kennytm
added
S-waiting-on-review
src/librustc/ty/query/mod.rs
Outdated
| [] fn privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Lrc<AccessLevels>, | ||
| [] fn check_privacy: CheckPrivacy(CrateNum) -> (), |
There was a problem hiding this comment.
Can you rename this to additional_privacy_checking? It seems like most of the actual privacy checking is done in check_mod_privacy so I think it would be confusing if a query called check_privacy did not actually do everything.
There was a problem hiding this comment.
I renamed it to check_private_in_public instead.
| @@ -87,7 +88,86 @@ note: required by `Foo` | |||
| LL | trait Foo { | |||
| | ^^^^^^^^^ | |||
|
|
|||
| error: aborting due to 2 previous errors | |||
| error[E0275]: overflow evaluating the requirement `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>: Foo` | |||
There was a problem hiding this comment.
I wonder if this constitutes an error reporting regression. It seems like a lot of spam. cc @estebank
(do we have a GH team for diagnostics questions?)
There was a problem hiding this comment.
There's already a similar error in this file. This just happens in two locations now since we do them in parallel and don't stop at the first one.
There was a problem hiding this comment.
I wonder if this constitutes an error reporting regression
I would say it does. In the past for similar situations where there's no way of reworking the current diagnostic plumbing, a way we've avoided duplicated errors is by keeping around a RefCell<HashSet> in the Session keeping track of all the Span/NodeId/HirId that have already been emitted and avoid emitting them a second time.
It seems like a lot of spam.
It does, and I'd go as far as saying that the current error is already quite bad, even without it triggering multiple times. There have been some efforts in the past to identify cycles and make the errors less verbose, but this case of NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> isn't yet handled, and it should.
I understand @Zoxc if you don't want to deal with these things in this PR, but would you mind filing follow up issues for the two points raised above if you don't, just so we don't lose track of them?
(do we have a GH team for diagnostics questions?)
We don't and we probably should, as there're quite a few of us now. I'll bring it up the next compiler meeting.
| )* | ||
| if let Some(panic) = panic { | ||
| ::std::panic::resume_unwind(panic); | ||
| } |
There was a problem hiding this comment.
What's going on here? Is this to make behaviour more consistent with parallel execution?
There was a problem hiding this comment.
Yes. Otherwise the single threaded compiler will stop at the first fatal error and the test output will diverge from the parallel compiler.
…ions required for AccessLevels
…guarantees that it will run immediately
|
📌 Commit 7985c6f has been approved by |
bors
added
S-waiting-on-bors
Centril
added a commit
to Centril/rust
that referenced
this pull request
Mar 9, 2019
…ister Refactor passes and pass execution to be more parallel For `syntex_syntax` (with 16 threads and 8 cores): - Cuts `misc checking 1` from `0.096s` to `0.08325s`. - Cuts `misc checking 2` from `0.3575s` to `0.2545s`. - Cuts `misc checking 3` from `0.34625s` to `0.21375s`. - Cuts `wf checking` from `0.3085s` to `0.05025s`. Reduces overall execution time for `syntex_syntax` (with 8 threads and cores) from `4.92s` to `4.34s`. Subsumes rust-lang#58494 Blocked on rust-lang#58250 r? @michaelwoerister
Centril
added a commit
to Centril/rust
that referenced
this pull request
Mar 9, 2019
…ister Refactor passes and pass execution to be more parallel For `syntex_syntax` (with 16 threads and 8 cores): - Cuts `misc checking 1` from `0.096s` to `0.08325s`. - Cuts `misc checking 2` from `0.3575s` to `0.2545s`. - Cuts `misc checking 3` from `0.34625s` to `0.21375s`. - Cuts `wf checking` from `0.3085s` to `0.05025s`. Reduces overall execution time for `syntex_syntax` (with 8 threads and cores) from `4.92s` to `4.34s`. Subsumes rust-lang#58494 Blocked on rust-lang#58250 r? @michaelwoerister
Centril
added a commit
to Centril/rust
that referenced
this pull request
Mar 9, 2019
…ister Refactor passes and pass execution to be more parallel For `syntex_syntax` (with 16 threads and 8 cores): - Cuts `misc checking 1` from `0.096s` to `0.08325s`. - Cuts `misc checking 2` from `0.3575s` to `0.2545s`. - Cuts `misc checking 3` from `0.34625s` to `0.21375s`. - Cuts `wf checking` from `0.3085s` to `0.05025s`. Reduces overall execution time for `syntex_syntax` (with 8 threads and cores) from `4.92s` to `4.34s`. Subsumes rust-lang#58494 Blocked on rust-lang#58250 r? @michaelwoerister
bors
added a commit
that referenced
this pull request
Mar 9, 2019
Rollup of 13 pull requests Successful merges: - #58518 (Use early unwraps instead of bubbling up errors just to unwrap in the end) - #58626 (rustdoc: add option to calculate "documentation coverage") - #58629 (rust-lldb: fix crash when printing empty string) - #58660 (MaybeUninit: add read_initialized, add examples) - #58670 (fixes #52482) - #58676 (look for python2 symlinks before bootstrap python) - #58679 (Refactor passes and pass execution to be more parallel) - #58750 (Make `Unique::as_ptr`, `NonNull::dangling` and `NonNull::cast` const) - #58762 (Mention `unwind(aborts)` in diagnostics for `#[unwind]`) - #58924 (Add as_slice() to slice::IterMut and vec::Drain) - #58990 (Actually publish miri in the manifest) - #59018 (std: Delete a by-definition spuriously failing test) - #59045 (Expose new_sub_parser_from_file) Failed merges: r? @ghost
37 tasks
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.
For
syntex_syntax(with 16 threads and 8 cores):misc checking 1from0.096sto0.08325s.misc checking 2from0.3575sto0.2545s.misc checking 3from0.34625sto0.21375s.wf checkingfrom0.3085sto0.05025s.Reduces overall execution time for
syntex_syntax(with 8 threads and cores) from4.92sto4.34s.Subsumes #58494
Blocked on #58250
r? @michaelwoerister