Skip to content

Conversation

Nadrieril
Copy link
Member

#79284 only moved code around but this changed inlining and caused a large perf regression. This fixes it for me, though I'm less confident than usual because the regression was not observable with my usual (i.e. incremental) compilation settings.

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 3, 2020
@jonas-schievink
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Dec 3, 2020

⌛ Trying commit 135550c934ca2d12b1590572ae7aaff8c8b69bdc with merge 26b0c16b67088a9dffa5b4fc0770ad8c8f51540c...

@bors
Copy link
Collaborator

bors commented Dec 3, 2020

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

@rust-timer
Copy link
Collaborator

Queued 26b0c16b67088a9dffa5b4fc0770ad8c8f51540c with parent 5be3f9f, future comparison URL.

@jyn514 jyn514 added A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 3, 2020
@Nadrieril Nadrieril force-pushed the fix-regression-79284 branch from 135550c to 793c40e Compare December 4, 2020 01:45
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (26b0c16b67088a9dffa5b4fc0770ad8c8f51540c): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

@Nadrieril
Copy link
Member Author

To compare with the regression. This more than fixes the regression on unicode_normalization, and almost fixes the one on match-stress-enum. (Remember that to undo 30% gain one only needs 23% loss). There was also #79523 in between that improved perf.

@jonas-schievink
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 4, 2020

📌 Commit 793c40e has been approved by jonas-schievink

@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 Dec 4, 2020
@bors
Copy link
Collaborator

bors commented Dec 4, 2020

⌛ Testing commit 793c40e with merge 2218520...

@bors
Copy link
Collaborator

bors commented Dec 4, 2020

☀️ Test successful - checks-actions
Approved by: jonas-schievink
Pushing 2218520 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 4, 2020
@bors bors merged commit 2218520 into rust-lang:master Dec 4, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 4, 2020
@Nadrieril Nadrieril deleted the fix-regression-79284 branch December 4, 2020 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns I-compiletime Issue: Problems and improvements with respect to compile times. 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.

8 participants