BTreeMap: consider leaf nodes to have empty edges

[ssomers]

Tried to make internal and leaf nodes more alike, because:

• The btree code is ambiguous about leaf edges (i.e., edges within leaf nodes). Iteration relies on them heavily, but some of the comments in node.rs suggest there are no leaf edges. In my definition of edges, internal edges contain a pointer to a child node, leaf edges don't.
• The btree code splits up / repeats some methods according to whether the node is leaf or internal, while the only difference is the kind of leaf edge contents.

r? @Mark-Simulacrum

[ssomers]

Some of the stuff split off into #77005. The rest is a little disappointing. The only way I found to make gdb_providers work is obnoxious. I couldn't python to access the InternalNode type, no matter how I try to wrap it.

Performance of insertion may be better for reasonably sized key/values.

 name                                           r3 ns/iter  r4 ns/iter  diff ns/iter   diff %  speedup
 btree::map::clone_fat_100                      45,305      52,676             7,371   16.27%   x 0.86
 btree::map::clone_fat_100_and_clear            45,755      51,880             6,125   13.39%   x 0.88
 btree::map::clone_fat_val_100                  23,963      27,126             3,163   13.20%   x 0.88
 btree::map::clone_fat_val_100_and_clear        23,618      27,295             3,677   15.57%   x 0.87
 btree::map::clone_fat_val_100_and_into_iter    34,394      38,174             3,780   10.99%   x 0.90
 btree::map::clone_slim_10k                     231,170     207,235          -23,935  -10.35%   x 1.12
 btree::map::clone_slim_10k_and_into_iter       230,603     207,045          -23,558  -10.22%   x 1.11
 btree::map::first_and_last_10k                 68          59                    -9  -13.24%   x 1.15
 btree::map::range_included_included            473,273     418,305          -54,968  -11.61%   x 1.13
 btree::set::difference_staggered_100_vs_100    694         905                  211   30.40%   x 0.77
 btree::set::difference_staggered_10k_vs_10k    68,588      85,926            17,338   25.28%   x 0.80
 btree::set::intersection_staggered_10k_vs_10k  61,094      68,490             7,396   12.11%   x 0.89
 btree::set::is_subset_100_vs_10k               1,197       1,338                141   11.78%   x 0.89

[ssomers]

Found tricks to simplify the gdb introspection. The result is still a bit disappointing but acceptable to me.

[Mark-Simulacrum]

I'll wait for #77005 to land before going ahead to review this.

[ssomers]

And I'll wait for #77005 to land to rebase.

[ssomers]

I think the unsafe (that I transplanted from LeafNode::new) is silly. The function does nothing unsafe. The value returned does not own any keys or values yet. I'm pretty sure that after wrapping it in BoxedNode, you could still wreak havoc without invoking any unsafe function.

[Mark-Simulacrum]

Hm, so I'm not sure this is a win. The node code is already pretty scattered (at least in my mind) -- and introducing another type parameter that readers need to figure out feels pretty bad, especially because it doesn't look like it really cleans anything up in a major way, just introduces additional indirection (at the type-level, at least).

Can you say more about the motivation for this? Perhaps there's an end goal in mind here that I'm not yet seeing?

[ssomers]

The motivation is in this PR's description. The first point can be covered by adapting comments, the second point is somewhat addressed by the leafy_ methods, but they complicate matters too because the length is adjusted before the edges are treated. I think the methods look better here, but the type overhead is more than I hoped.

[ssomers]

Or very concretely, if you try to treat edges in the split of internal nodes just like leafy_split treats the other arrays:

            ptr::copy_nonoverlapping(
                self.node.edge_at(self.idx + 1),
                MaybeUninit::slice_as_mut_ptr(&mut new_node.edges),
                new_len + 1,
            );

you end up realizing that the debug_assert in edge_at fails because length was already reduced. And it's not trivial to find out because any assert causes panic in drops, without a trace of the original problem in the stack. But that's another issue.

[ssomers]

Failing to find ways to make this more appealing