-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Build the query vtable directly. #90210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 138e96b with merge a06d7d3c5448c4be8d492913f6383d1be11a69be... |
☀️ Try build successful - checks-actions |
Queued a06d7d3c5448c4be8d492913f6383d1be11a69be with parent aa5740c, future comparison URL. |
Finished benchmarking commit (a06d7d3c5448c4be8d492913f6383d1be11a69be): comparison url. Summary: This change led to small relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
@@ -512,7 +512,7 @@ where | |||
|
|||
// First we try to load the result from the on-disk cache. | |||
// Some things are never cached on disk. | |||
if query.cache_on_disk(tcx, key) { | |||
if query.cache_on_disk { | |||
let prof_timer = tcx.dep_context().profiler().incr_cache_loading(); | |||
let result = query.try_load_from_disk(tcx, prev_dep_node_index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idly wondering if it might be an improvement to couple the Option in try_load_from_disk to cache_on_disk, such that we have something like cache_on_disk: fn(tcx, key) -> Option<fn(tcx, SerializedDepNodeIndex)>
which should let us avoid an extra bool and the expect()/Option wrapping.
But it's at best unclear whether that's actually any better, so probably not worth doing. I suppose the query.cache_on_disk cal here might as well be a if let Some(loader) = query.load_from_disk
, though, if we wanted to, and that's set to None if the given key isn't stored on disk by this compiler and so is pointless to try and load.
@bors r+ |
📌 Commit 138e96b has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (28d0e75): comparison url. Summary: This change led to small relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
Continuation of #89978.
This shrinks the query interface and attempts to reduce the amount of function pointer calls.