Simplify try_load_from_disk_fn.#155080
Conversation
It doesn't need to be in there, it can instead be at the single call site. Removing it eliminates one parameter, makes `define_queries!` smaller (which is always good), and also enables the next commit which tidies up profiling. This commit also changes how `value` and `verify` are initialized, because I don't like the current way of doing it after the declaration.
From `plumbing.rs` to `execution.rs`, which is where most of the other query profiling occurs. It also avoids eliminates some fn parameters. This means the `provided_to_erased` call in `try_from_load_disk_fn` is now included in the profiling when previously it wasn't. This is good because `provided_to_erased` is included in other profiling calls (e.g. calls to `invoke_provider_fn`).
|
|
|
I did this in two commits but it might be easier to review by looking at "Files changed" because of the way the second commit builds on the first. |
|
I have been informed that @Zalathar is busy with other things right now, let's try a different reviewer. |
|
@bors r+ |
Simplify `try_load_from_disk_fn`. `try_load_from_disk_fn` has a single call site. We can move some of the stuff within it to its single call site, which simplifies it, and also results in all of the query profiling code ending up in the same module. Details in individual commits. r? @Zalathar
|
@bors try @rust-timer queue Just in case :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Simplify `try_load_from_disk_fn`.
…uwer Rollup of 8 pull requests Successful merges: - #155047 (Always exhaustively match on typing mode) - #155080 (Simplify `try_load_from_disk_fn`.) - #152384 (Restrict EII declarations to functions at lowering time) - #153796 (Fix ICE when combining #[eii] with #[core::contracts::ensures]) - #154369 (Fix `pattern_from_macro_note` for bit-or expr) - #155027 ( Rename some more of our internal `#[rustc_*]` TEST attributes) - #155031 (delegation: fix unelided lifetime ICE, refactoring of GenericArgPosition) - #155040 (Fix code block whitespace handling in Markdown)
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (03c75e8): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary 0.2%, secondary -2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.4%, secondary 0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 488.4s -> 488.788s (0.08%) |
|
@bors rollup=maybe |
Rollup merge of #155080 - nnethercote:salvage, r=petrochenkov Simplify `try_load_from_disk_fn`. `try_load_from_disk_fn` has a single call site. We can move some of the stuff within it to its single call site, which simplifies it, and also results in all of the query profiling code ending up in the same module. Details in individual commits. r? @Zalathar
try_load_from_disk_fnhas a single call site. We can move some of the stuff within it to its single call site, which simplifies it, and also results in all of the query profiling code ending up in the same module. Details in individual commits.r? @Zalathar