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

BTreeMap: purge innocent use of into_kv_mut #75195

Merged
merged 1 commit into from
Aug 15, 2020

Conversation

ssomers
Copy link
Contributor

@ssomers ssomers commented Aug 5, 2020

Replace the use of into_kv_mut into more precise calls. This makes more sense if you know that the single remaining use of into_kv_mut is in fact evil and can be trialled in court (#75200) and sent to a correction facility (#73971).

No real performance difference reported (but functions that might benefit a tiny constant bit like BTreeMap::get_mut aren't benchmarked):

benchcmp old new --threshold 5
 name                       old ns/iter  new ns/iter  diff ns/iter  diff %  speedup
 btree::map::clone_fat_100  63,073       59,256             -3,817  -6.05%   x 1.06
 btree::map::iter_100       3,514        3,235                -279  -7.94%   x 1.09

@rust-highfive
Copy link
Collaborator

r? @hanna-kruppe

(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 Aug 5, 2020
@ssomers
Copy link
Contributor Author

ssomers commented Aug 5, 2020

(accidentally pressed some key too soon)

@ssomers
Copy link
Contributor Author

ssomers commented Aug 5, 2020

r? @Mark-Simulacrum

@ssomers ssomers force-pushed the btree_split_up_into_kv_mut branch from af4633a to b833c9d Compare August 5, 2020 17:12
@ssomers ssomers marked this pull request as draft August 5, 2020 17:31
@ssomers ssomers force-pushed the btree_split_up_into_kv_mut branch from b833c9d to 44154e1 Compare August 5, 2020 17:36
@ssomers ssomers marked this pull request as ready for review August 5, 2020 17:46
@ssomers ssomers force-pushed the btree_split_up_into_kv_mut branch 2 times, most recently from 7b9a772 to c360fb1 Compare August 9, 2020 12:22
@bors
Copy link
Contributor

bors commented Aug 11, 2020

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

@ssomers ssomers force-pushed the btree_split_up_into_kv_mut branch from c360fb1 to 3a02e06 Compare August 11, 2020 10:35
@ssomers ssomers changed the title BTreeMap: chop into_kv_mut into the two halves always desired BTreeMap: purge innocent use of into_kv_mut Aug 11, 2020
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 14, 2020

📌 Commit 3a02e06 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 Aug 14, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2020
Rollup of 17 pull requests

Successful merges:

 - rust-lang#73943 (Document the unsafe keyword)
 - rust-lang#74062 (deny(unsafe_op_in_unsafe_fn) in libstd/ffi/c_str.rs)
 - rust-lang#74185 (Remove liballoc unneeded explicit link)
 - rust-lang#74192 (Improve documentation on process::Child.std* fields)
 - rust-lang#74409 (Change Debug impl of SocketAddr and IpAddr to match their Display output)
 - rust-lang#75195 (BTreeMap: purge innocent use of into_kv_mut)
 - rust-lang#75214 (Use intra-doc links in `mem::manually_drop` & `mem::maybe_uninit`)
 - rust-lang#75432 (Switch to intra-doc links in `std::process`)
 - rust-lang#75482 (Clean up E0752 explanation)
 - rust-lang#75501 (Move to intra doc links in std::ffi)
 - rust-lang#75509 (Tweak suggestion for `this` -> `self`)
 - rust-lang#75511 (Do not emit E0228 when it is implied by E0106)
 - rust-lang#75515 (Bump std's libc version to 0.2.74)
 - rust-lang#75517 (Promotion and const interning comments)
 - rust-lang#75519 (BTreeMap: refactor splitpoint and move testing over to unit test)
 - rust-lang#75530 (Switch to intra-doc links in os/raw/*.md)
 - rust-lang#75531 (Migrate unit tests of btree collections to their native breeding ground)

Failed merges:

r? @ghost
@bors bors merged commit 83e75ac into rust-lang:master Aug 15, 2020
@ssomers ssomers deleted the btree_split_up_into_kv_mut branch August 15, 2020 08:27
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
…crum

 BTreeMap: introduce marker::ValMut and reserve Mut for unique access

The mutable BTreeMap iterators (apart from `DrainFilter`) are double-ended, meaning they have to rely on a front and a back handle that each represent a reference into the tree. Reserve a type category `marker::ValMut` for them, so that we guarantee that they cannot reach operations on handles with borrow type `marker::Mut`and that these operations can assume unique access to the tree.

Including rust-lang#75195, benchmarks report no genuine change:
```
benchcmp old new --threshold 5
 name                                 old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::iter_100                 3,333        3,023                -310   -9.30%   x 1.10
 btree::map::range_unbounded_vs_iter  36,624       31,569             -5,055  -13.80%   x 1.16
```

r? @Mark-Simulacrum
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
…crum

 BTreeMap: introduce marker::ValMut and reserve Mut for unique access

The mutable BTreeMap iterators (apart from `DrainFilter`) are double-ended, meaning they have to rely on a front and a back handle that each represent a reference into the tree. Reserve a type category `marker::ValMut` for them, so that we guarantee that they cannot reach operations on handles with borrow type `marker::Mut`and that these operations can assume unique access to the tree.

Including rust-lang#75195, benchmarks report no genuine change:
```
benchcmp old new --threshold 5
 name                                 old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::iter_100                 3,333        3,023                -310   -9.30%   x 1.10
 btree::map::range_unbounded_vs_iter  36,624       31,569             -5,055  -13.80%   x 1.16
```

r? @Mark-Simulacrum
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 4, 2020
 BTreeMap: introduce marker::ValMut and reserve Mut for unique access

The mutable BTreeMap iterators (apart from `DrainFilter`) are double-ended, meaning they have to rely on a front and a back handle that each represent a reference into the tree. Reserve a type category `marker::ValMut` for them, so that we guarantee that they cannot reach operations on handles with borrow type `marker::Mut`and that these operations can assume unique access to the tree.

Including rust-lang#75195, benchmarks report no genuine change:
```
benchcmp old new --threshold 5
 name                                 old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::iter_100                 3,333        3,023                -310   -9.30%   x 1.10
 btree::map::range_unbounded_vs_iter  36,624       31,569             -5,055  -13.80%   x 1.16
```

r? @Mark-Simulacrum
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
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.

6 participants