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.

Sign up for GitHub

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

Jump to bottom

Update rustc PGO benchmark list #97120

Merged
merged 1 commit into from
May 23, 2022
Merged

Update rustc PGO benchmark list #97120

merged 1 commit into from
May 23, 2022

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented May 17, 2022 •
edited
Loading

I noticed that the rustc PGO crates do not contain any crate that would stress the trait system. I tried adding and removing various crates to the PGO benchmark list here. Here's what I found:

  • Removing externs and match-stress regresses these two benchmarks by up to 15 % and removing them doesn't improve anything else, so we should keep them.
  • Adding keccak regresses diesel, otherwise it doesn't do much.
  • Adding tt-muncher improves it very slightly, not worth it to include it IMO.
  • Adding just diesel improves it by up to 1.5 % and others crate slightly, but regresses bitmaps.
  • Adding bitmaps improves both it and diesel, no other regressions.
  • Adding both bitmaps and diesel produces quite nice improvements and almost no regressions.
  • Adding ucd did not have a large effect on primary benchmarks.

r? @lqd

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented May 17, 2022

@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 17, 2022
@bors
Copy link
Contributor

bors commented May 17, 2022

⌛ Trying commit b2c40afdbbf2ac72cdc2f29d75a8ad34c817ef5b with merge cd37706ad459ee8ddfda4631be71120cb7eda19d...

@bors
Copy link
Contributor

bors commented May 17, 2022

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

@rust-timer
Copy link
Collaborator

Queued cd37706ad459ee8ddfda4631be71120cb7eda19d with parent 00755e4, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cd37706ad459ee8ddfda4631be71120cb7eda19d): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 7 17 86 28 93
mean2 0.6% 0.4% -0.5% -0.3% -0.4%
max 0.8% 0.8% -1.4% -0.4% -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.

@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 May 17, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented May 18, 2022 •
edited
Loading

The results are kind of mixed, diesel wins are not that great and most importantly, this regressed another crate which stresses the trait system, which sadly hints that we overfitted for diesel too much. I'll see what happens if we also add bitmaps to the PGO crates.

@Kobzol
Copy link
Contributor Author

Kobzol commented May 18, 2022

@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 18, 2022
@bors
Copy link
Contributor

bors commented May 18, 2022

⌛ Trying commit 826b56042c46222c23fe4cc68722005eb0ac9f59 with merge a6931fb09da1e3c532534832c652871039a931f1...

@bors
Copy link
Contributor

bors commented May 18, 2022

💔 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 May 18, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented May 18, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@bors
Copy link
Contributor

bors commented May 18, 2022

⌛ Trying commit 142c63269afc8bc434d9170b23e94420d1bdf23b with merge 4ffa399ed18788a359d66e57175212b8fad41506...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented May 18, 2022

💔 Test failed - checks-actions

@Kobzol Kobzol force-pushed the rustc-pgo-expansion branch from 142c632 to 6f8aae3 Compare May 18, 2022 09:41
@Kobzol
Copy link
Contributor Author

Kobzol commented May 18, 2022

@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 20, 2022
@bors
Copy link
Contributor

bors commented May 20, 2022

⌛ Trying commit 68e8b977944af1f67f4ee248d4ed7e6dce3b180a with merge 327cc09917311f65cf427e6c0bf5f7424af9fd05...

@bors
Copy link
Contributor

bors commented May 20, 2022

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

@rust-timer
Copy link
Collaborator

Queued 327cc09917311f65cf427e6c0bf5f7424af9fd05 with parent b5caa5a, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (327cc09917311f65cf427e6c0bf5f7424af9fd05): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 8 3 19 3
mean2 N/A 0.4% -0.3% -1.0% -0.3%
max N/A 0.6% -0.5% -2.1% -0.5%

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 🎉 relevant improvement found
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 1 0 0 1 1
mean2 2.6% N/A N/A -3.6% 2.6%
max 2.6% N/A N/A -3.6% 2.6%

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 1 7 1 2 2
mean2 2.1% 2.1% -3.1% -2.9% -0.5%
max 2.1% 2.6% -3.1% -3.8% -3.1%

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.

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

Footnotes

  1. number of relevant changes ↩ ↩2 ↩3

  2. the arithmetic mean of the percent change ↩ ↩2 ↩3

@rustbot rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels May 20, 2022
@Kobzol Kobzol marked this pull request as ready for review May 20, 2022 22:21
@Kobzol Kobzol changed the title [WIP] Experiment with rustc PGO benchmark list Update rustc PGO benchmark list May 20, 2022
@Kobzol
Copy link
Contributor Author

Kobzol commented May 20, 2022

r? @Mark-Simulacrum

I have changed the PR description with results of my findings. Based on them, I suggest to either add diesel and bitmaps to the PGO list, or leave the list as it is.

@rust-highfive rust-highfive assigned Mark-Simulacrum and unassigned lqd May 20, 2022
@apiraino apiraino added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label May 23, 2022
@Mark-Simulacrum
Copy link
Member

Let's add both; we'll also want to squash commits.

@Mark-Simulacrum Mark-Simulacrum removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2022
@Kobzol Kobzol force-pushed the rustc-pgo-expansion branch from 68e8b97 to a050d6b Compare May 23, 2022 15:55
@Kobzol
Copy link
Contributor Author

Kobzol commented May 23, 2022

Done, I have also turned off tidy line length check for the file, otherwise we would have to split the long benchmark string with some hacks or variables.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented May 23, 2022

📌 Commit a050d6b has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 23, 2022
@bors
Copy link
Contributor

bors commented May 23, 2022

⌛ Testing commit a050d6b with merge ee160f2...

@bors
Copy link
Contributor

bors commented May 23, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing ee160f2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2022
@bors bors merged commit ee160f2 into rust-lang:master May 23, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 23, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ee160f2): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 3 18 83 53 86
mean2 0.3% 0.8% -0.5% -0.5% -0.5%
max 0.4% 1.8% -1.3% -1.1% -1.3%

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 1 4 0 4 1
mean2 3.2% 1.8% N/A -1.4% 3.2%
max 3.2% 2.8% N/A -1.5% 3.2%

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 2 9 1 1 3
mean2 2.4% 2.8% -1.6% -2.8% 1.1%
max 2.7% 5.8% -1.6% -2.8% 2.7%

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 ↩3

  2. the arithmetic mean of the percent change ↩ ↩2 ↩3

@Kobzol Kobzol deleted the rustc-pgo-expansion branch October 25, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

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-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
10 participants
@Kobzol @rust-timer @bors @rust-log-analyzer @lqd @nnethercote @Mark-Simulacrum @apiraino @rust-highfive @rustbot