Skip to content

Commit 384f005

Browse files
committed
Update inner node builder to require two children
**Description** In a prior commit, I added a builder for creating inner nodes that guaranteed at least a single child. I realized later that I hadn't gone far enough, all inner nodes actually require at least two children! This commit updates the builder to add another type-state, modifies tests to pass this new requirement. **Motivation** In a separate branch I've been experimenting with some optimizations and one of those optimizations had a requirement that the inner node had at least two children. That made me remember the builder stuff and I figured it would be good to actually bring it up the real requirement. **Testing Done** Modified existing tests, no new, all passed.
1 parent 9b221e2 commit 384f005

8 files changed

Lines changed: 110 additions & 35 deletions

File tree

benches/node/match_prefix.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,23 @@ fn bench(c: &mut Criterion) {
2828
let p0 = &[0, 0, 0, 0, 0, 0, 0, 0];
2929
let node48_small = InnerNode48::<Box<[u8]>, usize, 16>::builder(p0, p0.len())
3030
.write_child(99, leaf_opaque)
31+
.write_child(100, leaf_opaque)
3132
.build();
3233
let node256_small = InnerNodeDirect::<Box<[u8]>, usize, 16>::builder(p0, p0.len())
3334
.write_child(99, leaf_opaque)
35+
.write_child(100, leaf_opaque)
3436
.build();
3537

3638
let p1 = &[
3739
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
3840
];
3941
let node48_large = InnerNode48::<Box<[u8]>, usize, 16>::builder(p1, p1.len())
4042
.write_child(99, leaf_opaque)
43+
.write_child(100, leaf_opaque)
4144
.build();
4245
let node256_large = InnerNodeDirect::<Box<[u8]>, usize, 16>::builder(p1, p1.len())
4346
.write_child(99, leaf_opaque)
47+
.write_child(100, leaf_opaque)
4448
.build();
4549

4650
macro_rules! generate_benches {

benches/node/min_max.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ fn bench(c: &mut Criterion) {
1414
let idx = i * skip;
1515
let node = InnerNode48::<CString, usize, 16>::builder(&[], 0)
1616
.write_child(idx, dangling_opaque)
17+
.write_child(idx.wrapping_add(1), dangling_opaque)
1718
.build();
1819
(idx, node)
1920
})
@@ -23,6 +24,7 @@ fn bench(c: &mut Criterion) {
2324
let idx = i * skip;
2425
let node = InnerNodeDirect::<CString, usize, 16>::builder(&[], 0)
2526
.write_child(idx, dangling_opaque)
27+
.write_child(idx.wrapping_add(1), dangling_opaque)
2628
.build();
2729
(idx, node)
2830
})

benches/tree/iter.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ fn iter_node<const PREFIX_LEN: usize, M: Measurement, N: InnerNode<PREFIX_LEN>>(
1313
) {
1414
for (idx, size) in sizes.iter().enumerate() {
1515
assert!(
16-
*size > 0,
17-
"size {size} in index {idx} must be greater than zero"
16+
*size >= 2,
17+
"size {size} in index {idx} must be at least two"
1818
);
1919
}
2020

@@ -28,7 +28,9 @@ fn iter_node<const PREFIX_LEN: usize, M: Measurement, N: InnerNode<PREFIX_LEN>>(
2828
let mut group = c.benchmark_group(format!("iter_node/{ty}"));
2929
for size in sizes {
3030
let mut iter = bytes.choose_multiple(&mut rng, *size as usize);
31-
let mut builder = N::builder(&[], 0).write_child(*iter.next().unwrap(), dangling_opaque);
31+
let mut builder = N::builder(&[], 0)
32+
.write_child(*iter.next().unwrap(), dangling_opaque)
33+
.write_child(*iter.next().unwrap(), dangling_opaque);
3234
for key in iter {
3335
builder = builder.write_child(*key, dangling_opaque);
3436
}
@@ -46,7 +48,7 @@ fn iter_node<const PREFIX_LEN: usize, M: Measurement, N: InnerNode<PREFIX_LEN>>(
4648
}
4749

4850
fn bench(c: &mut Criterion) {
49-
iter_node::<16, _, InnerNode4<CString, usize, 16>>(c, "n4", &[1, 4]);
51+
iter_node::<16, _, InnerNode4<CString, usize, 16>>(c, "n4", &[2, 4]);
5052
iter_node::<16, _, InnerNode16<CString, usize, 16>>(c, "n16", &[5, 12, 16]);
5153
iter_node::<16, _, InnerNode48<CString, usize, 16>>(c, "n48", &[17, 32, 48]);
5254
iter_node::<16, _, InnerNodeDirect<CString, usize, 16>>(c, "n256", &[49, 100, 152, 204, 256]);

src/raw/representation.rs

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -557,15 +557,18 @@ pub trait InnerNode<const PREFIX_LEN: usize>:
557557
/// Marker type for [`InnerNodeBuilder`]: no children have been added yet.
558558
pub struct NoChild;
559559

560-
/// Marker type for [`InnerNodeBuilder`]: at least one child has been added.
560+
/// Marker type for [`InnerNodeBuilder`]: exactly one child has been added.
561+
pub struct HasOneChild;
562+
563+
/// Marker type for [`InnerNodeBuilder`]: at least two children have been added.
561564
pub struct HasChild;
562565

563-
/// Typestate builder for inner nodes that enforces the non-empty invariant.
566+
/// Typestate builder for inner nodes that enforces the two-child minimum
567+
/// invariant.
564568
///
565569
/// The only way to call [`build`][InnerNodeBuilder::build] is to first add at
566-
/// least one child via
567-
/// [`write_child`][InnerNodeBuilder::write_child]. This is checked at
568-
/// compile time via the `S` typestate parameter.
570+
/// least two children via [`write_child`][InnerNodeBuilder::write_child]. This
571+
/// is checked at compile time via the `S` typestate parameter.
569572
///
570573
/// Obtain a builder via [`InnerNodeCommon::builder`].
571574
#[expect(clippy::type_complexity)]
@@ -578,8 +581,27 @@ impl<K, V, const PREFIX_LEN: usize, N> InnerNodeBuilder<K, V, PREFIX_LEN, N, NoC
578581
where
579582
N: InnerNodeCommon<K, V, PREFIX_LEN>,
580583
{
581-
/// Add the first child, transitioning the builder to the [`HasChild`] state
582-
/// and enabling [`build`][InnerNodeBuilder::build].
584+
/// Add the first child, transitioning the builder to the [`HasOneChild`]
585+
/// state.
586+
pub fn write_child(
587+
mut self,
588+
key_byte: u8,
589+
child: OpaqueNodePtr<K, V, PREFIX_LEN>,
590+
) -> InnerNodeBuilder<K, V, PREFIX_LEN, N, HasOneChild> {
591+
self.node.write_child(key_byte, child);
592+
InnerNodeBuilder {
593+
node: self.node,
594+
_state: PhantomData,
595+
}
596+
}
597+
}
598+
599+
impl<K, V, const PREFIX_LEN: usize, N> InnerNodeBuilder<K, V, PREFIX_LEN, N, HasOneChild>
600+
where
601+
N: InnerNodeCommon<K, V, PREFIX_LEN>,
602+
{
603+
/// Add the second child, transitioning the builder to the [`HasChild`]
604+
/// state and enabling [`build`][InnerNodeBuilder::build].
583605
pub fn write_child(
584606
mut self,
585607
key_byte: u8,
@@ -598,7 +620,31 @@ impl<K, V, const PREFIX_LEN: usize>
598620
{
599621
/// Add the first child to the node without bounds check or order.
600622
///
601-
/// This function transitions the build to the [`HasChild`] state and
623+
/// This function transitions the builder to the [`HasOneChild`] state.
624+
///
625+
/// # Safety
626+
/// - This functions assumes that the write is gonna be inbound (i.e the
627+
/// check for a full node is done previously to the call of this function)
628+
pub unsafe fn write_child_unchecked(
629+
mut self,
630+
key_byte: u8,
631+
child: OpaqueNodePtr<K, V, PREFIX_LEN>,
632+
) -> InnerNodeBuilder<K, V, PREFIX_LEN, InnerNode4<K, V, PREFIX_LEN>, HasOneChild> {
633+
// SAFETY: Covered by function safety requirements
634+
unsafe { self.node.write_child_unchecked(key_byte, child) };
635+
InnerNodeBuilder {
636+
node: self.node,
637+
_state: PhantomData,
638+
}
639+
}
640+
}
641+
642+
impl<K, V, const PREFIX_LEN: usize>
643+
InnerNodeBuilder<K, V, PREFIX_LEN, InnerNode4<K, V, PREFIX_LEN>, HasOneChild>
644+
{
645+
/// Add the second child to the node without bounds check or order.
646+
///
647+
/// This function transitions the builder to the [`HasChild`] state and
602648
/// enabling [`build`][InnerNodeBuilder::build].
603649
///
604650
/// # Safety
@@ -897,15 +943,19 @@ mod tests {
897943

898944
let n4 = InnerNode4::<Box<[u8]>, (), 16>::builder(&[], 0)
899945
.write_child(0, leaf_ptr)
946+
.write_child(1, leaf_ptr)
900947
.build();
901948
let n16 = InnerNode4::<Box<[u8]>, (), 16>::builder(&[], 0)
902949
.write_child(0, leaf_ptr)
950+
.write_child(1, leaf_ptr)
903951
.build();
904952
let n48 = InnerNode4::<Box<[u8]>, (), 16>::builder(&[], 0)
905953
.write_child(0, leaf_ptr)
954+
.write_child(1, leaf_ptr)
906955
.build();
907956
let n256 = InnerNode4::<Box<[u8]>, (), 16>::builder(&[], 0)
908957
.write_child(0, leaf_ptr)
958+
.write_child(1, leaf_ptr)
909959
.build();
910960

911961
let n4_ptr = const_addr(&n4 as *const InnerNode4<Box<[u8]>, (), 16>);
@@ -934,12 +984,15 @@ mod tests {
934984
.map(|leaf| NodePtr::from(leaf).to_opaque())
935985
.collect();
936986

937-
let mut node = N::builder(&[], 0).write_child(0, leaf_pointers[0]).build();
987+
let mut node = N::builder(&[], 0)
988+
.write_child(0, leaf_pointers[0])
989+
.write_child(1, leaf_pointers[1])
990+
.build();
938991

939992
assert!(!node.is_full());
940993

941-
for (idx, leaf_pointer) in leaf_pointers[1..].iter().copied().enumerate() {
942-
node.write_child(u8::try_from(idx + 1).unwrap(), leaf_pointer);
994+
for (idx, leaf_pointer) in leaf_pointers[2..].iter().copied().enumerate() {
995+
node.write_child(u8::try_from(idx + 2).unwrap(), leaf_pointer);
943996
}
944997

945998
for (idx, leaf_pointer) in leaf_pointers.iter().copied().enumerate() {
@@ -965,12 +1018,15 @@ mod tests {
9651018
.map(|leaf| NodePtr::from(leaf).to_opaque())
9661019
.collect();
9671020

968-
let mut node = N::builder(&[], 0).write_child(0, leaf_pointers[0]).build();
1021+
let mut node = N::builder(&[], 0)
1022+
.write_child(0, leaf_pointers[0])
1023+
.write_child(1, leaf_pointers[1])
1024+
.build();
9691025

9701026
assert!(!node.is_full());
9711027

972-
for (idx, leaf_pointer) in leaf_pointers[1..].iter().copied().enumerate() {
973-
node.write_child(u8::try_from(idx + 1).unwrap(), leaf_pointer);
1028+
for (idx, leaf_pointer) in leaf_pointers[2..].iter().copied().enumerate() {
1029+
node.write_child(u8::try_from(idx + 2).unwrap(), leaf_pointer);
9741030
}
9751031

9761032
for (idx, leaf_pointer) in leaf_pointers.iter().copied().enumerate() {
@@ -1005,10 +1061,13 @@ mod tests {
10051061
.map(|leaf| NodePtr::from(leaf).to_opaque())
10061062
.collect();
10071063

1008-
let mut node = N::builder(&[], 0).write_child(0, leaf_pointers[0]).build();
1064+
let mut node = N::builder(&[], 0)
1065+
.write_child(0, leaf_pointers[0])
1066+
.write_child(1, leaf_pointers[1])
1067+
.build();
10091068

1010-
for (idx, leaf_pointer) in leaf_pointers[1..].iter().copied().enumerate() {
1011-
node.write_child(u8::try_from(idx + 1).unwrap(), leaf_pointer);
1069+
for (idx, leaf_pointer) in leaf_pointers[2..].iter().copied().enumerate() {
1070+
node.write_child(u8::try_from(idx + 2).unwrap(), leaf_pointer);
10121071
}
10131072

10141073
let shrunk_node = node.shrink();
@@ -1039,10 +1098,13 @@ mod tests {
10391098
.map(|leaf| NodePtr::from(leaf).to_opaque())
10401099
.collect();
10411100

1042-
let mut node = N::builder(&[], 0).write_child(0, leaf_pointers[0]).build();
1101+
let mut node = N::builder(&[], 0)
1102+
.write_child(0, leaf_pointers[0])
1103+
.write_child(1, leaf_pointers[1])
1104+
.build();
10431105

1044-
for (idx, leaf_pointer) in leaf_pointers[1..].iter().copied().enumerate() {
1045-
node.write_child(u8::try_from(idx + 1).unwrap(), leaf_pointer);
1106+
for (idx, leaf_pointer) in leaf_pointers[2..].iter().copied().enumerate() {
1107+
node.write_child(u8::try_from(idx + 2).unwrap(), leaf_pointer);
10461108
}
10471109

10481110
assert_eq!(node.header().num_children(), num_children);

src/raw/representation/inner_node_direct.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ mod tests {
394394
let leaf_ptr = NodePtr::from(&mut leaf).to_opaque();
395395
let n = InnerNodeDirect::<Box<[u8]>, (), 16>::builder(&[], 0)
396396
.write_child(0, leaf_ptr)
397+
.write_child(1, leaf_ptr)
397398
.build();
398399

399400
n.grow();

src/raw/representation/inner_node_sorted.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,7 @@ mod tests {
672672
let leaf_ptr = NodePtr::from(&mut leaf).to_opaque();
673673
let n4 = InnerNode4::<Box<[u8]>, (), 16>::builder(&[], 0)
674674
.write_child(0, leaf_ptr)
675+
.write_child(1, leaf_ptr)
675676
.build();
676677

677678
n4.shrink();
@@ -747,8 +748,9 @@ mod tests {
747748

748749
let mut n16 = InnerNode16::<Box<[u8]>, (), 16>::builder(&[], 0)
749750
.write_child(0, v[0])
751+
.write_child(2, v[1])
750752
.build();
751-
for i in 1..16u8 {
753+
for i in 2..16u8 {
752754
n16.write_child(i * 2, v[usize::from(i)]);
753755
}
754756

src/raw/representation/pointers.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,15 +611,19 @@ mod tests {
611611

612612
let mut n4 = InnerNode4::<Box<[u8]>, usize, 16>::builder(&[], 0)
613613
.write_child(0, leaf_ptr)
614+
.write_child(1, leaf_ptr)
614615
.build();
615616
let mut n16 = InnerNode16::<Box<[u8]>, usize, 16>::builder(&[], 0)
616617
.write_child(0, leaf_ptr)
618+
.write_child(1, leaf_ptr)
617619
.build();
618620
let mut n48 = InnerNode48::<Box<[u8]>, usize, 16>::builder(&[], 0)
619621
.write_child(0, leaf_ptr)
622+
.write_child(1, leaf_ptr)
620623
.build();
621624
let mut n256 = InnerNodeDirect::<Box<[u8]>, usize, 16>::builder(&[], 0)
622625
.write_child(0, leaf_ptr)
626+
.write_child(1, leaf_ptr)
623627
.build();
624628

625629
let n4_ptr = NodePtr::from(&mut n4).to_opaque();

src/raw/visitor/well_formed.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -676,40 +676,38 @@ mod tests {
676676
let l2_ptr = NodePtr::allocate_node_ptr(l2, &Global);
677677
let l3_ptr = NodePtr::allocate_node_ptr(l3, &Global);
678678

679-
// Build n4_left with first child, then allocate
679+
// Build n4_left with both children upfront
680680
let n4_left = InnerNode4::builder(&[5, 6], 2)
681681
.write_child(1, l1_ptr.to_opaque())
682+
.write_child(2, l2_ptr.to_opaque())
682683
.build();
683684
let n4_left_ptr = NodePtr::allocate_node_ptr(n4_left, &Global);
684685

685-
// Build n4_right with first child (l3), then allocate; loop child added later
686+
// Build n4_right with l3 and a placeholder; loop child (root) added later
686687
let n4_right = InnerNode4::builder(&[7, 8], 2)
687688
.write_child(3, l3_ptr.to_opaque())
689+
.write_child(4, l3_ptr.to_opaque())
688690
.build();
689691
let n4_right_ptr = NodePtr::allocate_node_ptr(n4_right, &Global);
690692

691-
// Build n16 with first child (n4_left), then allocate; n4_right added later
693+
// Build n16 with n4_left and a placeholder; n4_right added later
692694
let n16 = InnerNode16::builder(&[1, 2], 2)
693695
.write_child(3, n4_left_ptr.to_opaque())
696+
.write_child(4, n4_left_ptr.to_opaque())
694697
.build();
695698

696699
// construct root early
697700
let root = NodePtr::allocate_node_ptr(n16, &Global);
698701

699-
{
700-
let n4_left = unsafe { n4_left_ptr.as_mut() };
701-
// Add remaining child
702-
n4_left.write_child(2, l2_ptr.to_opaque());
703-
}
704-
705702
{
706703
let n4_right = unsafe { n4_right_ptr.as_mut() };
707-
// replace normal l4 pointer with loop back to root
704+
// replace placeholder with loop back to root
708705
n4_right.write_child(4, root.to_opaque());
709706
}
710707

711708
{
712709
let n16 = unsafe { root.as_mut() };
710+
// replace placeholder with actual n4_right child
713711
n16.write_child(4, n4_right_ptr.to_opaque());
714712
}
715713

0 commit comments

Comments
 (0)