BTreeMap: reuse BoxedNode instances directly instead of their contents

[ssomers]

r? @Mark-Simulacrum

[ssomers]

Benchmarks pretend to be a bit faster, despite push/pop being relatively rare.

 name                                         old ns/iter  new ns/iter  diff ns/iter  diff %  speedup
 btree::map::clone_fat_100_and_into_iter      73,040       70,605             -2,435  -3.33%   x 1.03
 btree::map::insert_rand_10_000               33           31                     -2  -6.06%   x 1.06
 btree::map::insert_seq_10_000                96           90                     -6  -6.25%   x 1.07
 btree::map::iteration_mut_20                 69           66                     -3  -4.35%   x 1.05
 btree::set::clone_100_and_clear              1,833        1,776                 -57  -3.11%   x 1.03
 btree::set::clone_100_and_into_iter          1,820        1,764                 -56  -3.08%   x 1.03
 btree::set::clone_10k_and_remove_half        447,705      429,720           -17,985  -4.02%   x 1.04
 btree::set::intersection_random_10k_vs_100   2,153        2,085                 -68  -3.16%   x 1.03

[Mark-Simulacrum]

This feels like we're (potentially) doing needless work -- this looks equivalent to internal_node.edges[0].assume_init_read() which avoids the spurious write to edges -- can we use that instead?

(I suspect in practice LLVM probably ignores writing uninitialized data to some region of memory, but not sure).

[ssomers]

Indeed and I did the logical equivalent ptr::read on the other side.

[ssomers]

Swapped the 2 commits here and based on #77471 to avoid a benign conflict

[ssomers]

Nothing wrong, just easier to follow if I do the first commit separately.

[ssomers]

Still showing the same improvement, but now with the opposite on sets.

 name                                         old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::clone_slim_100                   2,043        2,106                  63    3.08%   x 0.97
 btree::map::clone_slim_100_and_clear         2,551        2,065                -486  -19.05%   x 1.24
 btree::map::clone_slim_100_and_drain_all     3,891        3,441                -450  -11.57%   x 1.13
 btree::map::clone_slim_100_and_drain_half    3,351        2,844                -507  -15.13%   x 1.18
 btree::map::clone_slim_100_and_into_iter     2,587        2,045                -542  -20.95%   x 1.27
 btree::map::clone_slim_100_and_pop_all       3,684        3,189                -495  -13.44%   x 1.16
 btree::map::clone_slim_100_and_remove_all    4,515        4,044                -471  -10.43%   x 1.12
 btree::map::clone_slim_100_and_remove_half   3,388        2,823                -565  -16.68%   x 1.20
 btree::set::clone_100_and_remove_all         3,397        3,792                 395   11.63%   x 0.90
 btree::set::clone_10k_and_remove_all         400,900      436,475            35,575    8.87%   x 0.92

[ssomers]

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

[Mark-Simulacrum]

@bors r+ rollup=never

[bors]

📌 Commit 184735a has been approved by Mark-Simulacrum

[Mark-Simulacrum]

Oh, hm, I just saw that #78104 lists itself as a better alternative to this. @bors r-

[ssomers]

Well I actually rebased #77244 on this anticipating it would arrive soon, so you approved it again…

It's mostly a logical alternative, explicitly not reusing rather than explicitly reusing, but it's easy enough to let #78104 overwrite this little change.

[Mark-Simulacrum]

Okay. I don't mind the extra PR. @bors r+

[bors]

📌 Commit 184735a has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit 184735a with merge 8682c9fe0f1a3307f310ee5e5b4a66f93c5f9466...

[ssomers]

#77244 was rebased on the same changes and made it through first. I wondered what bors would do and as far as I can tell it just turned away in disgust. The commit in #77244 is a different SHA-1 though, because it was rebased too.