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

Delay an is_local_ever_initialized call. #66537

Merged
merged 1 commit into from
Nov 22, 2019

Conversation

nnethercote
Copy link
Contributor

This commit moves the call after a return that almost always runs. It
speeds up the unicode_normalization benchmark by about 2%.

r? @spastorino

This commit moves the call after a `return` that almost always runs. It
speeds up the `unicode_normalization` benchmark by about 2%.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 19, 2019
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Nov 19, 2019

⌛ Trying commit 9651617 with merge b0ffe539b10a02cdb3ff9a220ea27b0b27154e2d...

@bors
Copy link
Contributor

bors commented Nov 19, 2019

☀️ Try build successful - checks-azure
Build commit: b0ffe539b10a02cdb3ff9a220ea27b0b27154e2d (b0ffe539b10a02cdb3ff9a220ea27b0b27154e2d)

@rust-timer
Copy link
Collaborator

Queued b0ffe539b10a02cdb3ff9a220ea27b0b27154e2d with parent 0ccee30, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit b0ffe539b10a02cdb3ff9a220ea27b0b27154e2d, comparison URL.

@Centril
Copy link
Contributor

Centril commented Nov 19, 2019

r? @pnkfelix

@rust-highfive rust-highfive assigned pnkfelix and unassigned spastorino Nov 19, 2019
@nnethercote
Copy link
Contributor Author

The CI perf results match the local ones: unicode_normalization instructions counts are reduced by up to 2.4%. Other benchmarks are unaffected.

@spastorino
Copy link
Member

spastorino commented Nov 19, 2019

Just talked to @Centril about this PR and we can continue with his comments in a follow up PR or issue.
@pnkfelix || @tmandry can you check that?.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 19, 2019

📌 Commit 9651617 has been approved by spastorino

@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 Nov 19, 2019
@spastorino
Copy link
Member

@bors rollup

@bors
Copy link
Contributor

bors commented Nov 19, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Nov 19, 2019

📌 Commit 9651617 has been approved by spastorino

@spastorino
Copy link
Member

spastorino commented Nov 19, 2019

@bors rollup=never

This is a performance improvement that may mask perf regressions if rolled up.

@bors
Copy link
Contributor

bors commented Nov 22, 2019

⌛ Testing commit 9651617 with merge 564f2d3...

bors added a commit that referenced this pull request Nov 22, 2019
…=spastorino

Delay an `is_local_ever_initialized` call.

This commit moves the call after a `return` that almost always runs. It
speeds up the `unicode_normalization` benchmark by about 2%.

r? @spastorino
@bors
Copy link
Contributor

bors commented Nov 22, 2019

☀️ Test successful - checks-azure
Approved by: spastorino
Pushing 564f2d3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 22, 2019
@bors bors merged commit 9651617 into rust-lang:master Nov 22, 2019
@nnethercote nnethercote deleted the delay-is_local_ever_initialized branch November 24, 2019 21:49
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.

7 participants