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

resolve/expand: Cache intermediate results of #[derive] expansion #82907

Merged
merged 2 commits into from
Apr 4, 2021

Conversation

petrochenkov
Copy link
Contributor

Expansion function for #[derive] (rustc_builtin_macros::derive::Expander::expand) may return an indeterminate result, and therefore can be called multiple times.
Previously we parsed the #[derive(Foo, Bar)]'s input and tried to resolve Foo and Bar on every such call.

Now we maintain a cache Resolver::derive_data and take all the necessary data from it if it was computed previously.
So Foo, Bar is now parsed at most once, and Foo and Bar are successfully resolved at most once.

@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 Mar 8, 2021
@petrochenkov
Copy link
Contributor Author

@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 Mar 8, 2021
@bors
Copy link
Contributor

bors commented Mar 8, 2021

⌛ Trying commit 45589d2e6e7af5866c804666bf52cbc0e671c2d5 with merge cc63cfbd3047bd17037ca21fa2ac23c7abfe4600...

@bors
Copy link
Contributor

bors commented Mar 8, 2021

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

@rust-timer
Copy link
Collaborator

Queued cc63cfbd3047bd17037ca21fa2ac23c7abfe4600 with parent 1d6b0f6, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (cc63cfbd3047bd17037ca21fa2ac23c7abfe4600): 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 Mar 9, 2021
@petrochenkov
Copy link
Contributor Author

The results are within the noise, but I'd still like to land this as a refactoring, because it will help with another caching-related FIXME in fn fully_expand_fragment which is more substantial.

@jyn514 jyn514 added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Name resolution I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 10, 2021
@bors

This comment has been minimized.

@bstrie bstrie added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 31, 2021
@petrochenkov
Copy link
Contributor Author

It's been almost a month.
r? @Aaron1011

@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 Apr 4, 2021
@petrochenkov
Copy link
Contributor Author

Updated.

@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2021
@Aaron1011
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 4, 2021

📌 Commit b965844 has been approved by Aaron1011

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021
@bors
Copy link
Contributor

bors commented Apr 4, 2021

⌛ Testing commit b965844 with merge 284954f5b9a7702c697585ee899ef91f68ac4dc1...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@Aaron1011
Copy link
Member

Spurious failure
@bors retry

@bors
Copy link
Contributor

bors commented Apr 4, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021
@bors
Copy link
Contributor

bors commented Apr 4, 2021

⌛ Testing commit b965844 with merge c755ee4...

@bors
Copy link
Contributor

bors commented Apr 4, 2021

☀️ Test successful - checks-actions
Approved by: Aaron1011
Pushing c755ee4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 4, 2021
@bors bors merged commit c755ee4 into rust-lang:master Apr 4, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Name resolution I-compiletime Issue: Problems and improvements with respect to compile times. 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. T-compiler Relevant 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.