Avoiding calling queries when collecting active queries #138672
+311
−148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
A-query-system
This comment has been minimized.
This comment has been minimized.
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
367760a to
fc76480
Compare
|
☔ The latest upstream changes (presumably #122156) made this pull request unmergeable. Please resolve the merge conflicts. |
|
r? oli-obk or fee1-dead? |
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
a88999f to
b482491
Compare
This comment has been minimized.
This comment has been minimized.
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
b482491 to
1c4bb4f
Compare
|
☔ The latest upstream changes (presumably #115747) made this pull request unmergeable. Please resolve the merge conflicts. |
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
1c4bb4f to
9f25677
Compare
|
☔ The latest upstream changes (presumably #138933) made this pull request unmergeable. Please resolve the merge conflicts. |
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
9f25677 to
f3448e6
Compare
|
☔ The latest upstream changes (presumably #138956) made this pull request unmergeable. Please resolve the merge conflicts. |
oli-obk
reviewed
Mar 26, 2025
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
f3448e6 to
23c63a9
Compare
This comment has been minimized.
This comment has been minimized.
Zoxc
force-pushed
the
deferred-queries-in-deadlock-handler
branch
from
23c63a9 to
6319bb3
Compare
|
Thanks! @bors r+ |
bors
added
S-waiting-on-bors
| /// but can later be changed to `QueryStackFrameExtra` containing concrete information | ||
| /// by calling `lift`. This is done so that collecting query does not need to invoke | ||
| /// queries, instead `lift` will call queries in a more appropriate location. | ||
| pub info: I, |
There was a problem hiding this comment.
Does the distinction between the lazy and evaluated frame extra need to be encoded into type system?
It causes the new generic parameter to spread over all of the query system.
Can it be an enum (or a dyn trait like LazyAttrTokenStream) instead of a generic?
If this is only used in query cycles, then it probably won't affect the query system performance on a good path.
There was a problem hiding this comment.
To make it an enum, we'd need to add 'tcx to the query system crate, so it could refer to QueryStackDeferred. That may spread further than the I parameter I used.
TaKO8Ki
added a commit
to TaKO8Ki/rust
that referenced
this pull request
Mar 26, 2025
…handler, r=oli-obk Avoiding calling queries when collecting active queries This PR changes active query collection to no longer call queries. Instead the fields needing queries have their computation delayed to when an cycle error is emitted or when printing the query backtrace in a panic. This is done by splitting the fields in `QueryStackFrame` needing queries into a new `QueryStackFrameExtra` type. When collecting queries `QueryStackFrame` will contain a closure that can create `QueryStackFrameExtra`, which does make use of queries. Calling `lift` on a `QueryStackFrame` or `CycleError` will convert it to a variant containing `QueryStackFrameExtra` using those closures. This also only calls queries needed to collect information on a cycle errors, instead of information on all active queries. Calling queries when collecting active queries is a bit odd. Calling queries should not be done in the deadlock handler at all. This avoids the out of memory scenario in rust-lang#124901.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 26, 2025
Rollup of 9 pull requests Successful merges: - rust-lang#130883 (Add environment variable query) - rust-lang#138672 (Avoiding calling queries when collecting active queries) - rust-lang#138702 (Allow spawning threads after TLS destruction) - rust-lang#138935 (Update wg-prio triagebot config) - rust-lang#138946 (Un-bury chapters from the chapter list in rustc book) - rust-lang#138964 (Implement lint against using Interner and InferCtxtLike in random compiler crates) - rust-lang#138977 (Don't deaggregate InvocationParent just to reaggregate it again) - rust-lang#138980 (Collect items referenced from var_debug_info) - rust-lang#138985 (Use the correct binder scope for elided lifetimes in assoc consts) r? `@ghost` `@rustbot` modify labels: rollup
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Mar 27, 2025
…handler, r=oli-obk Avoiding calling queries when collecting active queries This PR changes active query collection to no longer call queries. Instead the fields needing queries have their computation delayed to when an cycle error is emitted or when printing the query backtrace in a panic. This is done by splitting the fields in `QueryStackFrame` needing queries into a new `QueryStackFrameExtra` type. When collecting queries `QueryStackFrame` will contain a closure that can create `QueryStackFrameExtra`, which does make use of queries. Calling `lift` on a `QueryStackFrame` or `CycleError` will convert it to a variant containing `QueryStackFrameExtra` using those closures. This also only calls queries needed to collect information on a cycle errors, instead of information on all active queries. Calling queries when collecting active queries is a bit odd. Calling queries should not be done in the deadlock handler at all. This avoids the out of memory scenario in rust-lang#124901.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 27, 2025
Rollup of 10 pull requests Successful merges: - rust-lang#130883 (Add environment variable query) - rust-lang#138624 (Add mipsel maintainer) - rust-lang#138672 (Avoiding calling queries when collecting active queries) - rust-lang#138935 (Update wg-prio triagebot config) - rust-lang#138946 (Un-bury chapters from the chapter list in rustc book) - rust-lang#138964 (Implement lint against using Interner and InferCtxtLike in random compiler crates) - rust-lang#138977 (Don't deaggregate InvocationParent just to reaggregate it again) - rust-lang#138980 (Collect items referenced from var_debug_info) - rust-lang#138985 (Use the correct binder scope for elided lifetimes in assoc consts) - rust-lang#138987 (Always emit `native-static-libs` note, even if it is empty) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes active query collection to no longer call queries. Instead the fields needing queries have their computation delayed to when an cycle error is emitted or when printing the query backtrace in a panic.
This is done by splitting the fields in
QueryStackFrameneeding queries into a newQueryStackFrameExtratype. When collecting queriesQueryStackFramewill contain a closure that can createQueryStackFrameExtra, which does make use of queries. Callinglifton aQueryStackFrameorCycleErrorwill convert it to a variant containingQueryStackFrameExtrausing those closures.This also only calls queries needed to collect information on a cycle errors, instead of information on all active queries.
Calling queries when collecting active queries is a bit odd. Calling queries should not be done in the deadlock handler at all.
This avoids the out of memory scenario in #124901.