Streamline QueryVTableUnerased into GetQueryVTable#152841
Streamline QueryVTableUnerased into GetQueryVTable#152841Zalathar wants to merge 1 commit intorust-lang:mainfrom
QueryVTableUnerased into GetQueryVTable#152841Conversation
|
@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.
Streamline `QueryVTableUnerased` into `QueryVTableGetter`
|
| /// Marker type that implements [`QueryVTableGetter`] for this query. | ||
| pub(crate) enum VTableGetter {} | ||
|
|
||
| impl<'tcx> QueryVTableGetter<'tcx, FLAGS> for VTableGetter { |
There was a problem hiding this comment.
This is a trait with a single implementer (per query).
Would it be hard to refactor it into a free (per query) function somehow?
(Similarly to how restore_val was removed from the trait's interface.)
There was a problem hiding this comment.
Using a free function seems tricky, because the generics needed per callee would get more complicated, and that's something I'm trying to cut down on.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
|
Looks like I need to try some permutations to track down what’s triggering the perf changes. |
QueryVTableUnerased into QueryVTableGetterQueryVTableUnerased into GetQueryVTable
|
The current iteration only changes the trait itself and |
|
@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.
Streamline `QueryVTableUnerased` into `GetQueryVTable`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f16cfc0): comparison URL. Overall result: ❌✅ regressions and improvements - 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 countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 3.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 496.246s -> 482.423s (-2.79%) |
This comment has been minimized.
This comment has been minimized.
|
Let's see what perf looks like after removing @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.
Streamline `QueryVTableUnerased` into `GetQueryVTable`
|
r? nnethercote (or compiler) |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f8e0d28): comparison URL. Overall result: ✅ improvements - 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 countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.1%, secondary 4.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -3.0%)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: 479.41s -> 479.13s (-0.06%) |
|
@bors r+ |
|
These changes should have no perf effects, and the most recent perf run only shows noise, so this PR seems safe for rollup. @bors rollup=maybe |
Streamline `QueryVTableUnerased` into `GetQueryVTable` *[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust/pull/152841)* `QueryDispatcherUnerased` is an awkward name for an awkward trait. We can make it a bit more straightforward by removing its responsibility for erasing query values, and by observing that its only real responsibility beyond that is to know how to obtain the vtable for a particlar query from `tcx`.
Streamline `QueryVTableUnerased` into `GetQueryVTable` *[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust/pull/152841)* `QueryDispatcherUnerased` is an awkward name for an awkward trait. We can make it a bit more straightforward by removing its responsibility for erasing query values, and by observing that its only real responsibility beyond that is to know how to obtain the vtable for a particlar query from `tcx`.
Streamline `QueryVTableUnerased` into `GetQueryVTable` *[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust/pull/152841)* `QueryDispatcherUnerased` is an awkward name for an awkward trait. We can make it a bit more straightforward by removing its responsibility for erasing query values, and by observing that its only real responsibility beyond that is to know how to obtain the vtable for a particlar query from `tcx`.
…uwer Rollup of 15 pull requests Successful merges: - #152176 (Neon fast path for str::contains) - #152657 (std: move `exit` out of PAL) - #152841 (Streamline `QueryVTableUnerased` into `GetQueryVTable`) - #152845 (Skip `tidy` in PR CI jobs not dedicated to running `tidy`) - #152897 (Add optional json logging) - #153009 (Remove `rustc_feedable_queries` and `define_feedable` macros.) - #151558 (Port diagnostic attributes) - #152492 (mGCA: Enforce WF element types for array valtrees) - #152888 (Fix async drop glue MIR bug) - #152988 (Port `#[register_tool]` to the new attribute system) - #153018 (`unused_must_use` lint improvements) - #153023 (Update books) - #153033 (Clarify how "ensure" queries check whether they can skip execution) - #153043 (fix error on missing value for -C flags) - #153045 (rustc-dev-guide subtree update) Failed merges: - #153032 (Fix attribute parser and kind names.)
View all comments
QueryDispatcherUnerasedis an awkward name for an awkward trait.We can make it a bit more straightforward by removing its responsibility for erasing query values, and by observing that its only real responsibility beyond that is to know how to obtain the vtable for a particlar query from
tcx.