Access query (DepKind) metadata through fields#78452
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 73b774e23cce033a0a9c3582da8b84bd08b2bfcc with merge a0f145ad1a2c64f79fe055f2afa324f86e69fad1... |
|
☀️ Try build successful - checks-actions |
|
Queued a0f145ad1a2c64f79fe055f2afa324f86e69fad1 with parent 07e968b, future comparison URL. |
|
Finished benchmarking try commit (a0f145ad1a2c64f79fe055f2afa324f86e69fad1): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
The bug should be fixed now. |
|
@cjgillot does this need another perf run? Can you explain what went wrong before and what your fix was? |
|
Yes please. |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit e2baff32118d28a653a062be815659a306d77883 with merge 6e1160604446a0c65db29e950874ac85ec019752... |
jyn514
added
the
A-query-system
|
☀️ Try build successful - checks-actions |
|
Queued 6e1160604446a0c65db29e950874ac85ec019752 with parent ae9731c, future comparison URL. |
|
Finished benchmarking try commit (6e1160604446a0c65db29e950874ac85ec019752): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
There was a problem hiding this comment.
Could the fields be public so you can define your own DepKind outside rustc?
There was a problem hiding this comment.
This whole file should be moved outside rustc_middle eventually. However, there should be an exact correspondence between DepKinds, DepKindIndex and DEP_KINDS for encoding / decoding.
|
Perf looks like mostly minor regressions, with a small improvement on |
|
I have not found the origin of the regression yet. |
|
Removed |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
@cjgillot Am I correct that you believe you've resolved all of the comments I've left? |
|
@Mark-Simulacrum Yes. I rebased and added 3 commits to address your comments. |
|
r=me with the last 3 commits squashed into the appropriate prior commits |
|
@bors r=Mark-Simulacrum |
|
📌 Commit 0f334c3 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
| is_eval_always: false, | ||
|
|
||
| can_reconstruct_query_key: || true, | ||
| force_from_dep_node: |_, _| false, |
There was a problem hiding this comment.
TraitSelect and CompileCodegenUnit used to bug! on force_from_dep_node, as with Null and CrateMetadata. Was this change intentional? Sorry if this is obvious--I haven't read the whole PR.
There was a problem hiding this comment.
Actually, force_from_dep_node bailed out earlier (line 167), since those DepKinds are either anonymous, or with unreconstructible key. I was aiming at keeping the exact same behaviour, but panicking should be fine.
14 participants
This refactors the access to query definition metadata (attributes such as eval always, anon, has_params) and loading/forcing functions to generate a number of structs, instead of matching on the DepKind enum. This makes access to the fields cheaper to compile. Using a struct means that finding the metadata for a given query is just an offset away; previously the match may have been compiled to a jump table but likely not completely inlined as we expect here.
A previous attempt explored a similar strategy, but using trait objects in #78314 that proved less effective, likely due to higher overheads due to forcing dynamic calls and poorer cache utilization (all metadata is fairly densely packed with this PR).