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

Optimize BitIter #65425

Merged
merged 2 commits into from
Oct 16, 2019
Merged

Optimize BitIter #65425

merged 2 commits into from
Oct 16, 2019

Conversation

nnethercote
Copy link
Contributor

A minor speed improvement.

@rust-highfive
Copy link
Collaborator

r? @zackmdavis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 15, 2019
@zackmdavis
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 15, 2019

📌 Commit f5bfffd has been approved by zackmdavis

@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 15, 2019
@nnethercote
Copy link
Contributor Author

I added another commit, one that benefits from the faster BitIter. But I didn't see the r+ before that happened. I have now removed the additional commit; I will do it in a different PR.

@nnethercote
Copy link
Contributor Author

@bors r=zackmdavis

@bors
Copy link
Contributor

bors commented Oct 15, 2019

📌 Commit f5bfffd has been approved by zackmdavis

Centril added a commit to Centril/rust that referenced this pull request Oct 15, 2019
…kmdavis

Optimize `BitIter`

A minor speed improvement.
@Centril
Copy link
Contributor

Centril commented Oct 15, 2019

Failed in #65430 (comment), @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 15, 2019
This factors out some duplicated code.
This commit removes an `Option` check in `BitIter::next()`, avoids
calling `trailing_zeros()` when it's not necessary, and avoids the need
for `enumerate()`. This gives a tiny (0.2%) instruction count win on a
couple of benchmarks.

The commit also adds some comments, which is good because this iteration
code is moderately complex.
@nnethercote
Copy link
Contributor Author

I mistakenly thought it was ok for unsigned integers to wrap with +. wrapping_add fixes the problem.

@bors r=zackmdavis

@bors
Copy link
Contributor

bors commented Oct 15, 2019

📌 Commit 60851b0 has been approved by zackmdavis

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2019
tmandry added a commit to tmandry/rust that referenced this pull request Oct 15, 2019
…kmdavis

Optimize `BitIter`

A minor speed improvement.
bors added a commit that referenced this pull request Oct 15, 2019
Rollup of 14 pull requests

Successful merges:

 - #64603 (Reducing spurious unused lifetime warnings.)
 - #64623 (Remove last uses of gensyms)
 - #65235 (don't assume we can *always* find a return type hint in async fn)
 - #65242 (Fix suggestion to constrain trait for method to be found)
 - #65265 (Cleanup librustc mir err codes)
 - #65293 (Optimize `try_expand_impl_trait_type`)
 - #65307 (Try fix incorrect "explicit lifetime name needed")
 - #65308 (Add long error explanation for E0574)
 - #65353 (save-analysis: Don't ICE when resolving qualified type paths in struct members)
 - #65389 (Return `false` from `needs_drop` for all zero-sized arrays.)
 - #65402 (Add troubleshooting section to PGO chapter in rustc book.)
 - #65425 (Optimize `BitIter`)
 - #65438 (Organize `never_type`  tests)
 - #65444 (Implement AsRef<[T]> for List<T>)

Failed merges:

 - #65390 (Add long error explanation for E0576)

r? @ghost
@bors bors merged commit 60851b0 into rust-lang:master Oct 16, 2019
@nnethercote nnethercote deleted the optimize-BitIter branch October 16, 2019 18:41
bors added a commit that referenced this pull request Oct 16, 2019
Speed up `LexicalResolve::expansion()`

A couple of improvements that speed up `unicode_normalization` by about 4%. The first commit was enabled by the improvements to `BitSet` iteration in #65425.

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

Speed up `LexicalResolve::expansion()`

A couple of improvements that speed up `unicode_normalization` by about 4%. The first commit was enabled by the improvements to `BitSet` iteration in rust-lang#65425.

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

Speed up `LexicalResolve::expansion()`

A couple of improvements that speed up `unicode_normalization` by about 4%. The first commit was enabled by the improvements to `BitSet` iteration in rust-lang#65425.

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

Speed up `LexicalResolve::expansion()`

A couple of improvements that speed up `unicode_normalization` by about 4%. The first commit was enabled by the improvements to `BitSet` iteration in rust-lang#65425.

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

Speed up `LexicalResolve::expansion()`

A couple of improvements that speed up `unicode_normalization` by about 4%. The first commit was enabled by the improvements to `BitSet` iteration in rust-lang#65425.

r? @nikomatsakis
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