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

Test some panic!()-related perf things #92140

Closed
wants to merge 4 commits into from
Closed

Test some panic!()-related perf things #92140

wants to merge 4 commits into from

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Dec 20, 2021

Trying out some stuff on top of #92068 to track down a perf issue.

@m-ou-se m-ou-se added the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Dec 20, 2021
@m-ou-se m-ou-se self-assigned this Dec 20, 2021
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 20, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2021
@m-ou-se
Copy link
Member Author

m-ou-se commented Dec 20, 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 Dec 20, 2021
@m-ou-se m-ou-se removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2021
@bors
Copy link
Contributor

bors commented Dec 20, 2021

⌛ Trying commit 3c0d5ee with merge 8c6bc808c5e11716651cfe45f58547b97a96ff39...

@m-ou-se m-ou-se mentioned this pull request Dec 20, 2021
Merged
@bors
Copy link
Contributor

bors commented Dec 20, 2021

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

@rust-timer
Copy link
Collaborator

Queued 8c6bc808c5e11716651cfe45f58547b97a96ff39 with parent 23f6923, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8c6bc808c5e11716651cfe45f58547b97a96ff39): comparison url.

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

  • Moderate improvement in instruction counts (up to -1.3% on incr-unchanged builds of issue-88862)
  • Large regression in instruction counts (up to 3.2% on incr-patched: b9b3e592dd cherry picked builds of style-servo)

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 Dec 21, 2021
@m-ou-se m-ou-se mentioned this pull request Dec 21, 2021
Merged
@m-ou-se m-ou-se closed this Dec 21, 2021
@m-ou-se m-ou-se deleted the libcore2021 branch December 21, 2021 15:48
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2021
@bors
Auto merge of rust-lang#92155 - m-ou-se:panic-fn, r=eddyb
390bb34 
Use panic() instead of panic!() in some places in core.

See rust-lang#92068 and rust-lang#92140.

This avoids the `panic!()` macro in a few potentially hot paths. This becomes more relevant when switching `core` to Rust 2021, as it'll avoid format_args!() and save some compilation time. (It doesn't make a huge difference, but still.) (Also the errors in const panic become slightly nicer.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@m-ou-se m-ou-se

Labels
perf-regression Performance regression. S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@m-ou-se @rust-timer @bors @rust-highfive @rustbot @fee1-dead