Move rustc_query_system code, part 3#152516
Move rustc_query_system code, part 3#152516nnethercote wants to merge 6 commits intorust-lang:mainfrom
rustc_query_system code, part 3#152516Conversation
|
|
|
This PR moves most of what's left in |
|
|
||
| #[inline] | ||
| #[instrument(skip(tcx, dep_graph_data, result, hash_result, format_value), level = "debug")] | ||
| pub fn incremental_verify_ich<Tcx, V>( |
There was a problem hiding this comment.
Suggestion: Since we’re moving it anyway, this whole incremental_verify_ich chunk looks like a good candidate for its own submodule.
There was a problem hiding this comment.
rustc_query_system has a submodule ich and I was wondering about that, but to keep things simple I mostly moved things to modules of the same name when they existed.
There was a problem hiding this comment.
(I haven't gotten around to moving ich to rustc_middle yet.)
There was a problem hiding this comment.
From what I can tell, the ich module is the definition of StableHashingContext (used in many places), whereas incremental_verify_ich is some plumbing code that happens to use StableHashingContext. So I suspect that they are more-or-less separate from a module grouping perspective.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
I still don't understand what does exactly "plumbing" mean in context of the query system 😅
There was a problem hiding this comment.
I use plumbing as a general term for all the code that happens between when you call tcx.my_query(key), and when it actually calls the underlying query provider.
In terms of the modules named plumbing, I mostly interpret it as “here's the stuff that we didn't pull out into a more meaningfully-named module”, though it also tends to be where all the big complicated query-processing macros are.
There was a problem hiding this comment.
Just imagine it's called utils :P
24fad1b to
9906bae
Compare
|
I fixed the test failure. @Zalathar: I'm not sure how far the review process you are, and how strongly you want |
|
I’m pretty confident that it belongs in a submodule ( I’d strongly prefer not to move it straight into |
|
r=me with a |
9906bae to
5dd7978
Compare
|
Should be perf-neutral, let's check: @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Move `rustc_query_system` code, part 3
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (a868e96): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary -7.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -3.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 482.691s -> 481.14s (-0.32%) |
From `rustc_query_state` to `rustc_middle`.
This includes the types `QueryInfo`, `QueryJob`, `QueryJobId`, `QueryWaiter`, `QueryLatch`, and `QueryLatchInfo`. `CycleError` and `QueryStack*` had to come along too, due to type interdependencies. The `QueryStack*` types are put into a new submodule `rustc_middle::query::stack`.
This one is straightforward.
From `rustc_query_system` to `rustc_middle`.
From `rustc_query_system::query::plumbing` to `rustc_middle::query::plumbing`.
Most of the files within the `dep_graph` module can be moved wholesale into `rustc_middle`. But two of them (`mod.rs` and `dep_node.rs`) have the same name as existing files in `rustc_middle`, so for those I just copied the contents into the existing files. The commit also moves `QueryContext` and `incremental_verify_ich*` because they are tightly intertwined with the dep graph code. And a couple of error structs moved as well.
5dd7978 to
ee391fc
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Now with a @bors r=Zalathar |
|
@bors rollup=maybe |
Following on from #152419.
r? @Zalathar