Skip to content

Allow recovering symbol_name query when there are no substs - #92503

Closed
Aaron1011 wants to merge 1 commit into
rust-lang:masterfrom
Aaron1011:symbol-name-recovery
Closed

Allow recovering symbol_name query when there are no substs#92503
Aaron1011 wants to merge 1 commit into
rust-lang:masterfrom
Aaron1011:symbol-name-recovery

Conversation

@Aaron1011

Copy link
Copy Markdown
Contributor

This is accomplished by creating a new symbol_name_for_plain_item
query, which is automatically called when symbol_name is invoked
with a supported key. The symbol_name_for_plain_item query just
uses a DefId as its argument, so it can be recovered.

This is accomplished by creating a new `symbol_name_for_plain_item`
query, which is automatically called when `symbol_name` is invoked
with a supported key. The `symbol_name_for_plain_item` query just
uses a `DefId` as its argument, so it can be recovered.
@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 2, 2022
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @petrochenkov

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 2, 2022
@Aaron1011

Copy link
Copy Markdown
ContributorAuthor

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

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

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 2, 2022
@bors

bors commented Jan 2, 2022

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 86d3b47 with merge 99632a0f277a6b08405c1e1f9c9b5587840fdac4...

@bors

bors commented Jan 3, 2022

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: 99632a0f277a6b08405c1e1f9c9b5587840fdac4 (99632a0f277a6b08405c1e1f9c9b5587840fdac4)

@rust-timer

Copy link
Copy Markdown
Collaborator

Queued 99632a0f277a6b08405c1e1f9c9b5587840fdac4 with parent 8f3238f, future comparison URL.

@petrochenkovpetrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 3, 2022
@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (99632a0f277a6b08405c1e1f9c9b5587840fdac4): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 3, 2022
@petrochenkov

Copy link
Copy Markdown
Contributor

I'm missing the context here.

Is this extracted from some other PR?
I remember seeing something like this before.

What does "recovering" mean in this context?

@petrochenkovpetrochenkov 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 Jan 4, 2022
@Aaron1011

Aaron1011 commented Jan 4, 2022

Copy link
Copy Markdown
ContributorAuthor

When a query input changes (i.e. the input gets marked red), the query system will try to re-run the query to see if the result is unchanged. If so, then the query can be marked green, even though one of its inputs is red.

The process of re-running the query is called 'forcing', and it depends on us being able to reconstruct the original query input ('recovery') to pass to the query. Currently, this is only supported for DefId/LocalDefId/CrateNum, since we use the DefPathHash as the DepNode hash.

We could try extending recovery to other query types, but that would require decoding of the type to be fallible (I think trying to decode a stale Ty will currently panic if some of the data is not present in the current session).

This was split out from #92204. However, I think #92533 is a much better approach (it's actually a perf improvement), so there's probably no need for this PR for the foreseeable future.

@petrochenkov

Copy link
Copy Markdown
Contributor

Ok, this also seems to be a minor regression, so I'll keep this assigned to you for closing or doing something else.

@michaelwoerister

Copy link
Copy Markdown
Member

You could try another perf run with cache_on_disk_if { true } for the new query.

@JohnCSimonJohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2022
@JohnCSimonJohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 6, 2022
@camelidcamelid added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 2, 2022
@camelid

Copy link
Copy Markdown
Member

triage: Hi, I'm closing this PR since it looks like that was already suggested due to perf issues, and it's been inactive for a while. Please feel free to re-open at any time if you'd like to work on it further!

@camelidcamelid closed this Aug 2, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-inactiveStatus: Inactive and waiting on the author. This is often applied to closed PRs.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@Aaron1011@rust-highfive@rust-timer@bors@petrochenkov@michaelwoerister@camelid@JohnCSimon@rustbot