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.