Skip to content
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

Reduce the amount of untracked state in TyCtxt #85153

Merged
merged 12 commits into from
Jun 1, 2021
Merged

Conversation

cjgillot
Copy link
Contributor

Access to untracked global state may generate instances of #84970.

The GlobalCtxt contains the lowered HIR, the resolver outputs and interners.
By wrapping the resolver inside a query, we make sure those accesses are properly tracked.
As a no_hash query, all dependent queries essentially become eval_always,
what they should have been from the beginning.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 10, 2021
@rust-log-analyzer

This comment has been minimized.

@cjgillot
Copy link
Contributor Author

r? @Aaron1011

@petrochenkov petrochenkov self-assigned this May 10, 2021
@Aaron1011
Copy link
Member

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 10, 2021
@bors
Copy link
Contributor

bors commented May 10, 2021

⌛ Trying commit e7f85d5d56e67d76769f003cb66fd2c93aa8d66d with merge 570c16c920c6bcfc3414b8d72b0aac02b812c715...

@bors
Copy link
Contributor

bors commented May 10, 2021

☀️ Try build successful - checks-actions
Build commit: 570c16c920c6bcfc3414b8d72b0aac02b812c715 (570c16c920c6bcfc3414b8d72b0aac02b812c715)

@rust-timer
Copy link
Collaborator

Queued 570c16c920c6bcfc3414b8d72b0aac02b812c715 with parent 266f452, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (570c16c920c6bcfc3414b8d72b0aac02b812c715): 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 rollup- to bors.

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 10, 2021
@michaelwoerister
Copy link
Member

michaelwoerister commented May 11, 2021

Because I see it being used here: StableVec looks problematic to me because its HashStable implementation "hides" the order of its items, but that order is visible to users of the vec. I think this type should be removed and replaced with either a hash set (if the order does not matter) or a regular Vec (which must be built in a deterministic way).

@michaelwoerister
Copy link
Member

It looks like the trait map is the only code that uses StableVec, so this PR would be a good opportunity to get rid of it.

@petrochenkov petrochenkov removed their assignment May 11, 2021
@bors
Copy link
Contributor

bors commented May 11, 2021

☔ The latest upstream changes (presumably #82272) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented May 12, 2021

☔ The latest upstream changes (presumably #83610) made this pull request unmergeable. Please resolve the merge conflicts.

@Aaron1011 Aaron1011 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2021
@cjgillot cjgillot deleted the qresolve branch June 1, 2021 20:02
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2021
Only compute the trait map once

Part of rust-lang#85153

r? `@Aaron1011`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2021
Make is_private_dep a query.

Part of rust-lang#85153

r? `@Aaron1011`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2021
Restrict access to crate_name.

Also remove original_crate_name, which had the exact same implementation.

Part of rust-lang#85153
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 3, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 10, 2021
Drop metadata_encoding_version.

Part of rust-lang#85153

r? `@Aaron1011`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2021
Make allocator_kind a query.

Part of rust-lang#85153

r? `@Aaron1011`
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Jul 7, 2021
Drop metadata_encoding_version.

Part of rust-lang#85153

r? `@Aaron1011`
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Jul 7, 2021
Make allocator_kind a query.

Part of rust-lang#85153

r? `@Aaron1011`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2021
Reduce the amount of untracked state in TyCtxt -- Take 2

Main part of rust-lang#85153

The offending line (rust-lang#85153 (comment)) is replaced by a FIXME until the possible bug and the perf concern are both resolved.

r? `@Aaron1011`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.