Skip to content

initialize Predicate and Ty interners with higher capacity #90743

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

Closed
wants to merge 2 commits into from

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Nov 9, 2021

No description provided.

@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(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 Nov 9, 2021
@the8472
Copy link
Member Author

the8472 commented Nov 9, 2021

@bors try @rust-timer queue

Expected outcome: faster check builds, possible regression in max-rss by a fixed amount per rustc invocation (not code-dependent)

@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 Nov 9, 2021
@bors
Copy link
Collaborator

bors commented Nov 9, 2021

⌛ Trying commit 98a2032 with merge 9869d54de4acbf66ee1cc59bfcc173c0be8993c3...

@bors
Copy link
Collaborator

bors commented Nov 10, 2021

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

@rust-timer
Copy link
Collaborator

Queued 9869d54de4acbf66ee1cc59bfcc173c0be8993c3 with parent d608229, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9869d54de4acbf66ee1cc59bfcc173c0be8993c3): comparison url.

Summary: This change led to moderate relevant improvements 🎉 in compiler performance.

  • Moderate improvement in instruction counts (up to -0.4% on full builds of regression-31157)

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

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 10, 2021
@the8472 the8472 marked this pull request as draft November 10, 2021 15:36
@the8472
Copy link
Member Author

the8472 commented Nov 10, 2021

max-rss impact is bigger than expected. page faults too.
I'll try a more aggressive resizing strategy instead of pre-allocation. 2-3% of cpu cycles when checking core are spent on resizing those hash tables.

@the8472 the8472 force-pushed the interning-set-capacity branch from bfef8fd to 1cbfe5d Compare November 13, 2021 23:46
@the8472
Copy link
Member Author

the8472 commented Nov 13, 2021

@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 Nov 13, 2021
@bors
Copy link
Collaborator

bors commented Nov 13, 2021

⌛ Trying commit 1cbfe5d7020401d9dfa6c17e32124748d8d951a6 with merge e205bcd83c263da56a7592bb671f1327d1369bed...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Nov 14, 2021

💔 Test failed - checks-actions

@bors bors 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 Nov 14, 2021
@the8472 the8472 force-pushed the interning-set-capacity branch from 1cbfe5d to 1c294f1 Compare November 14, 2021 00:02
@rust-log-analyzer

This comment has been minimized.

@the8472
Copy link
Member Author

the8472 commented Nov 14, 2021

@bors try

@bors
Copy link
Collaborator

bors commented Nov 14, 2021

⌛ Trying commit 1c294f1 with merge 6c099d0e977ff048ef53e45db7b43c1807268134...

@bors
Copy link
Collaborator

bors commented Nov 14, 2021

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

@rust-timer
Copy link
Collaborator

Queued 6c099d0e977ff048ef53e45db7b43c1807268134 with parent b416e38, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6c099d0e977ff048ef53e45db7b43c1807268134): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

  • Moderate improvement in instruction counts (up to -0.5% on full builds of coercions)
  • Very large regression in instruction counts (up to 5.3% on incr-unchanged builds of inflate)

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.

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

@rustbot rustbot added perf-regression Performance regression. 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 Nov 14, 2021
@the8472 the8472 closed this Nov 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants