Skip to content

Conversation

ssomers
Copy link
Contributor

@ssomers ssomers commented Mar 18, 2021

A possible addition to / partial replacement of #83147: no longer linearly search the upper bound of a range in the initial portion of the keys we already know are below the lower bound.

  • Should be faster: fewer key comparisons at the cost of some instructions dealing with offsets
  • Makes code a little more complicated.
  • No longer detects ill-defined Ord implementations, but that wasn't a publicised feature, and was quite incomplete, and was only done in the range and range_mut methods.
    r? @Mark-Simulacrum

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

ssomers commented Mar 18, 2021

Comparing performance (with various versions of this) always seems to confirm that avoiding the superfluous comparisons pays off in range_included_included, but to my amazement sets back range_included_excluded. No other changes above noise.

 btree::map::range_included_excluded            465,055         490,340               25,285   5.44%   x 0.95
 btree::map::range_included_included            476,255         433,940              -42,315  -8.88%   x 1.10
 btree::map::range_included_unbounded           150,614         149,596               -1,018  -0.68%   x 1.01
 btree::map::range_unbounded_unbounded          76,012          76,010                    -2  -0.00%   x 1.00
 btree::map::range_unbounded_vs_iter            81,179          81,175                    -4  -0.00%   x 1.00

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

Seems reasonable.

@bors
Copy link
Collaborator

bors commented Apr 4, 2021

📌 Commit fd6e4e4 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 Apr 4, 2021
@bors
Copy link
Collaborator

bors commented Apr 4, 2021

⌛ Testing commit fd6e4e4 with merge 88e7862...

@bors
Copy link
Collaborator

bors commented Apr 4, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 4, 2021
@bors bors merged commit 88e7862 into rust-lang:master Apr 4, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 4, 2021
@ssomers ssomers deleted the btree_prune_range_search_overlap branch May 4, 2021 11:29
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.

5 participants