Make the Query enum a simple struct.#80891
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. |
|
⌛ Trying commit e08e9742ba5989cdb6b27f51226311a2e6372d5a with merge dea0e5624588437b1a358664ff4c776fae7d9625... |
|
☀️ Try build successful - checks-actions |
|
Queued dea0e5624588437b1a358664ff4c776fae7d9625 with parent a2cd91c, future comparison URL. @rustbot label: +S-waiting-on-perf |
|
Finished benchmarking try commit (dea0e5624588437b1a358664ff4c776fae7d9625): 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 |
|
@matthewjasper Do you intend to review this or do you want it reassigned? |
|
This should be reassigned, I'm don't really understand the query system implementation well enough to review this. |
|
While I review, let's kick off a more recent perf run - it seems like it's worthwhile to see if some of the mixed results (and minor regressions to bootstrap builds) from the previous one have gone away now. @bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit e08e9742ba5989cdb6b27f51226311a2e6372d5a with merge e332f2214ecfdd09201163667071ec79884b208f... |
Mark-Simulacrum
left a comment
There was a problem hiding this comment.
This seems interesting but at least with the previous performance results not really worth it.
I left some feedback, but it's all mostly minor things and asking for documentation.
I do think it might be worth exploring either reusing or defining a Key trait and storing the Query struct as something roughly like this which might help mitigate some of the performance problems while still giving us a more readable/maintainable implementation.
struct Query<'a> {
name: &'static str,
key: &'a dyn Key,
}
This comment has been minimized.
This comment has been minimized.
The call to `ty::print::with_forced_impl_filename_line` is done when constructing the description, at the construction of the QueryStackFrame.
|
@bors r+ (Feel free to do this yourself too, if you're just rebasing) |
|
📌 Commit ea9100f216e2fa276d9fc84dbc1f06c2e81b2bb8 has been approved by |
This comment has been minimized.
This comment has been minimized.
|
@bors r=Mark-Simulacrum |
|
📌 Commit 903f65f has been approved by |
|
⌛ Testing commit 903f65f with merge c32beb4dd2a6d8e1783ba1b66999d2e1757f2ca5... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
|
I can't reproduce the failure. |
|
☀️ Test successful - checks-actions |
A lot of code in
rustc_query_systemis generic over it, only to encode an exceptional error case: query cycles.The delayed computations are now done at cycle detection.