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

Gather LLVM PGO profiles from rustc-perf suite on real-world crates #94704

Merged
merged 2 commits into from
Mar 13, 2022

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Mar 7, 2022

This PR expands the benchmark suite used to gather LLVM PGO profiles in CI from libcore to several real-world crates. I hand-picked a few crates, but the list is up for debate.

Previous results that we got from running syn,cargo,serde looked pretty good.

Running libcore + rustc-perf with some number of crates is repeated now (and for BOLT it will also be needed), so maybe we can extract it to a bash function?

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 7, 2022
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

I think extracting the running of perf into a function is a good step to take, yeah. I would be interested in seeing if we get similar results from just Full benchmarks (rather than running the full set of incremental benchmarks as well).

@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 7, 2022

Okay. Currently, this PR has expanded the list of executed crates w.r.t. the previous PR. Do you want to run perf for this or should I remove the incremental builds right away?

@Mark-Simulacrum
Copy link
Member

@bors try @rust-timer queue

Yeah, let's kick off a perf build, and then we can drop incremental and try again.

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

bors commented Mar 7, 2022

⌛ Trying commit ffa5976 with merge 6513a04b37f3b91602d705d5a18fcb1bf5983559...

@bors
Copy link
Contributor

bors commented Mar 7, 2022

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

@rust-timer
Copy link
Collaborator

Queued 6513a04b37f3b91602d705d5a18fcb1bf5983559 with parent ecb867e, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6513a04b37f3b91602d705d5a18fcb1bf5983559): comparison url.

Summary: This benchmark run shows 79 relevant improvements 🎉 to instruction counts.

  • Arithmetic mean of relevant improvements: -3.0%
  • Arithmetic mean of all relevant changes: -3.0%
  • Largest improvement in instruction counts: -5.7% on incr-full builds of webrender debug

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

Kobzol commented Mar 8, 2022

Here is a comparison of syn,cargo,serde vs syn,cargo,serde,ripgrep,regex,clap-rs,hyper-2.

There is a slight improvement, but we are also hitting diminishing returns at this point. Dist build took 1h 21 minutes, which is similar to previous CI times.

@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 8, 2022

I created a bash function (it's my first one, so go easy on me please :D ) and changed it to only run Full runs for LLVM PGO.

@rust-log-analyzer

This comment has been minimized.

@Kobzol Kobzol force-pushed the llvm-pgo-update-suite branch from c48e3dd to e90e883 Compare March 8, 2022 10:02
@lqd
Copy link
Member

lqd commented Mar 8, 2022

Just as an over-fitting sanity check: it would be cool to have a short summary of the effect this has on some crates we don't track in the benchmark suite. The perf runs at least show the benefits on the crates that are not explicitly profiled for PGO.

I can help with that, let me know when this is ready to try.

@Aaron1011
Copy link
Member

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

bors commented Mar 8, 2022

⌛ Trying commit e90e883 with merge ca29313890801dc91b8e5937e78d5d3546557874...

@bors
Copy link
Contributor

bors commented Mar 8, 2022

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

@Mark-Simulacrum
Copy link
Member

@rust-timer build ca29313890801dc91b8e5937e78d5d3546557874

@rust-timer
Copy link
Collaborator

Queued ca29313890801dc91b8e5937e78d5d3546557874 with parent 1eb7258, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ca29313890801dc91b8e5937e78d5d3546557874): comparison url.

Summary: This benchmark run shows 81 relevant improvements 🎉 to instruction counts.

  • Arithmetic mean of relevant improvements: -2.8%
  • Largest improvement in instruction counts: -5.5% on incr-full builds of webrender debug

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

Kobzol commented Mar 9, 2022

Here is a comparsion of All runs (full + all incr. modes) vs only Full runs.

It seems to be mostly the same for instruction counts, but there are some larger regressions in cycle counts (not sure how relevant that is). Since it doesn't seem like running also incr modes adds a lot of time to CI, I would suggest to keep all of the runs in CI. WDYT?

@Kobzol Kobzol mentioned this pull request Mar 10, 2022
8 tasks
@Mark-Simulacrum
Copy link
Member

This seems good to me. I think we can probably endlessly iterate on the benchmark selection, but this shows a solid improvement already and that seems like it should be good enough to me.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 13, 2022

📌 Commit e90e883 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2022
@bors
Copy link
Contributor

bors commented Mar 13, 2022

⌛ Testing commit e90e883 with merge ebed06f...

@bors
Copy link
Contributor

bors commented Mar 13, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 13, 2022
@bors bors merged commit ebed06f into rust-lang:master Mar 13, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 13, 2022
@Kobzol Kobzol deleted the llvm-pgo-update-suite branch March 13, 2022 18:35
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ebed06f): comparison url.

Summary: This benchmark run shows 83 relevant improvements 🎉 to instruction counts.

  • Arithmetic mean of relevant improvements: -2.8%
  • Largest improvement in instruction counts: -5.6% on incr-full builds of style-servo debug

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

@rustbot label: -perf-regression

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants