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

BTree: move more shared iterator code into navigate.rs #81937

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

ssomers
Copy link
Contributor

@ssomers ssomers commented Feb 9, 2021

The functions in navigate.rs only exist to support iterators, and these look easier on my eyes if there is a shared struct with the recurring pair of handles.

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2021
@ssomers
Copy link
Contributor Author

ssomers commented Feb 10, 2021

benchcmp old new --threshold 10
 name                                           old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::first_and_last_0                   10           12                      2   20.00%   x 0.83
 btree::map::first_and_last_10k                 60           68                      8   13.33%   x 0.88
 btree::map::iter_1                             11,139       7,022              -4,117  -36.96%   x 1.59
 btree::map::iter_100                           12,293       7,775              -4,518  -36.75%   x 1.58
 btree::map::iter_10k                           13,294       9,029              -4,265  -32.08%   x 1.47
 btree::map::iter_1m                            14,408       9,924              -4,484  -31.12%   x 1.45
 btree::map::range_included_included            418,226      503,045            84,819   20.28%   x 0.83
 btree::map::range_unbounded_vs_iter            126,138      81,175            -44,963  -35.65%   x 1.55
 btree::set::difference_random_100_vs_10k       3,179        2,443                -736  -23.15%   x 1.30
 btree::set::difference_staggered_100_vs_10k    2,236        2,743                 507   22.67%   x 0.82
 btree::set::intersection_random_100_vs_10k     2,779        2,271                -508  -18.28%   x 1.22
 btree::set::intersection_random_10k_vs_100     2,785        2,251                -534  -19.17%   x 1.24
 btree::set::intersection_staggered_100_vs_10k  2,039        2,550                 511   25.06%   x 0.80
 btree::set::is_subset_100_vs_10k               1,180        1,639                 459   38.90%   x 0.72

@bors
Copy link
Contributor

bors commented Feb 12, 2021

☔ The latest upstream changes (presumably #81486) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Feb 14, 2021

☔ The latest upstream changes (presumably #81956) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Feb 15, 2021

☔ The latest upstream changes (presumably #82103) made this pull request unmergeable. Please resolve the merge conflicts.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never p=0

@bors
Copy link
Contributor

bors commented Feb 20, 2021

📌 Commit 342aa69 has been approved by Mark-Simulacrum

@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 Feb 20, 2021
@bors
Copy link
Contributor

bors commented Feb 22, 2021

⌛ Testing commit 342aa69 with merge 3cc54d0993aa1b56887dce7fcb95ed279a79dba2...

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Build completed successfully in 0:01:03
+ /scripts/validate-toolstate.sh
Cloning into 'rust-toolstate'...
<Nothing changed>
error: miri maintainer @oli-obk is not assignable in the rust-lang/rust repo
error: rustfmt maintainer @topecongiro is not assignable in the rust-lang/rust repo
error: nomicon maintainer @Gankra is not assignable in the rust-lang/rust repo
error: reference maintainer @matthewjasper is not assignable in the rust-lang/rust repo
error: reference maintainer @Havvy is not assignable in the rust-lang/rust repo
error: reference maintainer @ehuss is not assignable in the rust-lang/rust repo
error: embedded-book maintainer @therealprof is not assignable in the rust-lang/rust repo
error: edition-guide maintainer @ehuss is not assignable in the rust-lang/rust repo
error: rustc-dev-guide maintainer @amanjeev is not assignable in the rust-lang/rust repo

  To be assignable, a person needs to be explicitly listed as a
  collaborator in the repository settings. The simple way to
  fix this is to ask someone with 'admin' privileges on the repo
  to add the person or whole team as a collaborator with 'read'
  privileges. Those privileges don't grant any extra permissions
  so it's safe to apply them.
The build will fail due to this.

@bors
Copy link
Contributor

bors commented Feb 22, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 22, 2021
@ehuss
Copy link
Contributor

ehuss commented Feb 22, 2021

@bors retry
error: miri maintainer @oli-obk is not assignable in the rust-lang/rust repo

@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 Feb 22, 2021
@bors
Copy link
Contributor

bors commented Feb 22, 2021

⌛ Testing commit 342aa69 with merge b10e76598262dd4ef9112cdc9a00e79126e37f0d...

@bors
Copy link
Contributor

bors commented Feb 22, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 22, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@ehuss
Copy link
Contributor

ehuss commented Feb 22, 2021

@bors retry
dist-x86_64-mingw failed mysteriously without a logfile after 1 hour.

@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 Feb 22, 2021
@bors
Copy link
Contributor

bors commented Feb 23, 2021

⌛ Testing commit 342aa69 with merge b02a619...

@bors
Copy link
Contributor

bors commented Feb 23, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing b02a619 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 23, 2021
@bors bors merged commit b02a619 into rust-lang:master Feb 23, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 23, 2021
@ssomers ssomers deleted the btree_drainy_refactor_9b branch February 23, 2021 08:59
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 20, 2021
…rk-Simulacrum

BTree: encapsulate LeafRange better & some debug asserts

Looking at iterators again, I think rust-lang#81937 didn't house enough code in `LeafRange`. Moving the API boundary a little makes things more local in navigate.rs and less complicated in map.rs.

r? `@Mark-Simulacrum`
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