Skip to content

Conversation

nnethercote
Copy link
Contributor

A win for unicode_normalization.

r? @nikomatsakis

Some places use the local `tcx` variable, some use `self.tcx()`. This
commit removes the local variable so that all places use `self.tcx()`,
for consistency.
This wins 6% on `unicode_normalization`, by avoiding a call to
`lub_concrete_regions()` and a `Region` equality test.
The function only has one call site, so we don't need a tag argument.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2019
@nnethercote
Copy link
Contributor Author

I tried a whole lot of different things to speed this up, but commit 2 was the only thing that helped. Commits 1 and 3 are just minor clean-ups.

@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@bors
Copy link
Collaborator

bors commented Oct 10, 2019

⌛ Trying commit 8cd25e7 with merge bb3defd56b91fd479b0d8deffea23a68f3936982...

@bors
Copy link
Collaborator

bors commented Oct 10, 2019

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

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems ok but I will probably have to undo it shortly, if I land #65232. Well, perhaps adapt it is more correct -- there's probably a shortened test that would be equally efficient.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Oct 10, 2019

r=me once perf measurements are in

@nnethercote
Copy link
Contributor Author

The rust-timer queue command seems to have failed. Let's try it the old-fashioned way:

@rust-timer build bb3defd56b91fd479b0d8deffea23a68f3936982

@rust-timer
Copy link
Collaborator

Queued bb3defd56b91fd479b0d8deffea23a68f3936982 with parent aa45e03, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit bb3defd56b91fd479b0d8deffea23a68f3936982, comparison URL.

@nnethercote
Copy link
Contributor Author

Up to 7% wins on unicode_normalization, and negligible effects on other benchmarks.

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Oct 11, 2019

📌 Commit 8cd25e7 has been approved by nikomatsakis

@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 Oct 11, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 14, 2019
…-expansion, r=nikomatsakis

Optimize `LexicalResolve::expansion`.

A win for `unicode_normalization`.

r? @nikomatsakis
tmandry added a commit to tmandry/rust that referenced this pull request Oct 15, 2019
…-expansion, r=nikomatsakis

Optimize `LexicalResolve::expansion`.

A win for `unicode_normalization`.

r? @nikomatsakis
bors added a commit that referenced this pull request Oct 15, 2019
Rollup of 10 pull requests

Successful merges:

 - #65170 (rustc_metadata: Privatize private code and remove dead code)
 - #65260 (Optimize `LexicalResolve::expansion`.)
 - #65261 (Remove `Option` from `TokenStream`)
 - #65332 (std::fmt: reorder docs)
 - #65340 (Several changes to the codegen backend organization)
 - #65365 (Include const generic arguments in metadata)
 - #65398 (Bring attention to suggestions when the only difference is capitalization)
 - #65410 (syntax: add parser recovery for intersection- / and-patterns `p1 @ p2`)
 - #65415 (Remove an outdated test output file)
 - #65416 (Minor sync changes)

Failed merges:

r? @ghost
@bors bors merged commit 8cd25e7 into rust-lang:master Oct 15, 2019
@nnethercote nnethercote deleted the optimize-LexicalResolve-expansion branch October 15, 2019 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants