Skip to content

store codegen_fn_attrs in crate metadata#96473

Merged
bors merged 5 commits into
rust-lang:masterfrom
lcnr:querify-codegen-fn-attrs
May 9, 2022
Merged

store codegen_fn_attrs in crate metadata#96473
bors merged 5 commits into
rust-lang:masterfrom
lcnr:querify-codegen-fn-attrs

Conversation

@lcnr

@lcnr lcnr commented Apr 27, 2022

Copy link
Copy Markdown
Contributor

extracted from #95562 because the change isn't trivial.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 27, 2022
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @fee1-dead

(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 Apr 27, 2022
@lcnr

lcnr commented Apr 27, 2022

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
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 Apr 27, 2022
@bors

bors commented Apr 27, 2022

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 00da420d86e9c4c61da6aa3f22f3d853aa971794 with merge 8bda55c77d599ffd1ee91eba6c0dfa8d31734149...

Comment thread compiler/rustc_typeck/src/collect.rs Outdated
@bors

bors commented Apr 27, 2022

Copy link
Copy Markdown
Collaborator

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

@rust-timer

Copy link
Copy Markdown
Collaborator

Queued 8bda55c77d599ffd1ee91eba6c0dfa8d31734149 with parent bb85bca, future comparison URL.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (8bda55c77d599ffd1ee91eba6c0dfa8d31734149): comparison url.

Summary:

  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: 😿 relevant regressions found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 37 32 0 2 37
mean2 0.6% 1.0% N/A -0.2% 0.6%
max 1.4% 4.6% N/A -0.2% 1.4%

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 may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Apr 27, 2022
@lcnr

lcnr commented Apr 27, 2022

Copy link
Copy Markdown
Contributor Author

maybe caching it on disk if its already simply pulled from the metadata isn't too useful

@bors try @rust-timer queue

@rust-timer

Copy link
Copy Markdown
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 Apr 27, 2022
@bors

bors commented Apr 27, 2022

Copy link
Copy Markdown
Collaborator

⌛ Trying commit de08cbe2840ae1761852a1a984627c82640b605e with merge 95de7c6d4d85ca601c2838decc62bacf341ca265...

@rust-timer

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

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

@bors

bors commented Apr 27, 2022

Copy link
Copy Markdown
Collaborator

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

@rust-timer

Copy link
Copy Markdown
Collaborator

Queued 95de7c6d4d85ca601c2838decc62bacf341ca265 with parent bb85bca, future comparison URL.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (95de7c6d4d85ca601c2838decc62bacf341ca265): comparison url.

Summary:

  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: 😿 relevant regressions found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 38 28 1 2 39
mean2 0.6% 1.0% -0.3% -0.2% 0.5%
max 1.5% 4.6% -0.3% -0.2% 1.5%

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 may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 27, 2022
@lcnr

lcnr commented Apr 27, 2022

Copy link
Copy Markdown
Contributor Author

alright, this performance decrease seems acceptable, considering that with #95562 we can now remove inline and some other somewhat frequent attributes from the crate metadata which should offset this regression

@lcnr

lcnr commented Apr 29, 2022

Copy link
Copy Markdown
Contributor Author

r? rust-lang/compiler

@rust-highfive rust-highfive assigned cjgillot and unassigned fee1-dead Apr 29, 2022
@cjgillot

Copy link
Copy Markdown
Contributor

Normally, I'd be all for more queries, but the perf result is a bit too large for it's own good. It looks like it is dominated by metadata encoding.

It may be more interesting to land the refactoring in #95562 that makes it possible to omit attributes.
Once this is done, we will be able to identify the cost of the tradeoff in #96473 (comment).

@lcnr

lcnr commented May 9, 2022

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot 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 May 9, 2022
@klensy

klensy commented May 9, 2022

Copy link
Copy Markdown
Contributor

Finished benchmarking commit (1ceb86c): comparison url.

Improved all, except doc builds (if uncheck all profiles except doc, it can be clearly visible), which regressed.

@cjgillot

cjgillot commented May 9, 2022

Copy link
Copy Markdown
Contributor

r=me with the warning removed.

@lcnr lcnr force-pushed the querify-codegen-fn-attrs branch from 9dc70c0 to 32b13ac Compare May 9, 2022 16:40
@lcnr

lcnr commented May 9, 2022

Copy link
Copy Markdown
Contributor Author

@bors r=cjgillot

@bors

bors commented May 9, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 32b13ac has been approved by cjgillot

@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 May 9, 2022
@bors

bors commented May 9, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 32b13ac with merge 88860d5...

@bors

bors commented May 9, 2022

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 88860d5 to master...

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (88860d5): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 6 28 89 39 95
mean2 0.3% 0.6% -0.5% -1.6% -0.4%
max 0.6% 2.2% -1.4% -4.8% -1.4%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@lcnr lcnr deleted the querify-codegen-fn-attrs branch May 10, 2022 06:00
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 4, 2023
Fix outdated comment of `fn_can_unwind`

The first part is outdated since rust-lang#96473, and the second part is outdated since rust-lang#97235
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. 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.

9 participants