WIP: Unit value optimizations

src/counters.rs

@@ -250,6 +250,146 @@ pub(crate) fn record_make_unique(cloned: bool) {
     }
 }
 
+/// The key-byte capacity of a single [LineListNode](crate::line_list_node), and the resulting node size
+pub const LIST_NODE_KEY_BYTES: usize = crate::line_list_node::KEY_BYTES_CNT;
+/// The size in bytes of a single `LineListNode`, derived from [LIST_NODE_KEY_BYTES]
+pub const LIST_NODE_SIZE: usize = crate::line_list_node::LINE_LIST_NODE_SIZE;
+
+/// Memory attribution for a trie, broken down by node type
+///
+/// Walks physical (deduplicated) nodes, so structurally shared subtries are counted once. Use it to
+/// decide where layout work pays: the split between list-node and dense-node bytes varies enormously
+/// with key shape, and is what determines whether a given optimization is worth anything.
+#[derive(Clone, Default, Debug)]
+pub struct MemProfile {
+    pub list_nodes: usize,
+    pub list_bytes: usize,
+    pub dense_nodes: usize,
+    pub dense_items: usize,
+    pub dense_cap: usize,
+    pub dense_bytes: usize,
+    pub cell_nodes: usize,
+    pub cell_items: usize,
+    pub cell_bytes: usize,
+    pub klen_hist: Vec<usize>,
+    pub val_slots: usize,
+    pub child_slots: usize,
+    pub both_slots: usize,
+    pub key_bytes_used: usize,
+    pub at_cap: usize,
+    pub empty_nodes: usize,
+    pub dangling_slots: usize,
+    pub dense_val_slots: usize,
+    pub node_klen_hist: Vec<usize>,
+    pub allval_nodes: usize,
+    pub allval_klen_hist: Vec<usize>,
+}
+impl MemProfile {
+    fn merge(mut self, o: Self) -> Self {
+        self.list_nodes += o.list_nodes; self.list_bytes += o.list_bytes;
+        self.dense_nodes += o.dense_nodes; self.dense_items += o.dense_items; self.dense_bytes += o.dense_bytes; self.dense_cap += o.dense_cap;
+        self.cell_nodes += o.cell_nodes; self.cell_items += o.cell_items; self.cell_bytes += o.cell_bytes;
+        self.val_slots += o.val_slots; self.child_slots += o.child_slots;
+        self.both_slots += o.both_slots; self.key_bytes_used += o.key_bytes_used; self.at_cap += o.at_cap;
+        if self.klen_hist.len() < o.klen_hist.len() { self.klen_hist.resize(o.klen_hist.len(), 0); }
+        for (i, c) in o.klen_hist.iter().enumerate() { self.klen_hist[i] += c; }
+        if self.node_klen_hist.len() < o.node_klen_hist.len() { self.node_klen_hist.resize(o.node_klen_hist.len(), 0); }
+        for (i, c) in o.node_klen_hist.iter().enumerate() { self.node_klen_hist[i] += c; }
+        if self.allval_klen_hist.len() < o.allval_klen_hist.len() { self.allval_klen_hist.resize(o.allval_klen_hist.len(), 0); }
+        for (i, c) in o.allval_klen_hist.iter().enumerate() { self.allval_klen_hist[i] += c; }
+        self.allval_nodes += o.allval_nodes;
+        self.empty_nodes += o.empty_nodes; self.dangling_slots += o.dangling_slots;
+        self.dense_val_slots += o.dense_val_slots;
+        self
+    }
+    pub fn total_bytes(&self) -> usize { self.list_bytes + self.dense_bytes + self.cell_bytes }
+    pub fn report_list_slots(&self) {
+        let tot: usize = self.klen_hist.iter().sum();
+        println!("  list slot key-length histogram ({} slots, {} value-slots, {} child-slots):", tot, self.val_slots, self.child_slots);
+        let mut acc = 0;
+        for (len, cnt) in self.klen_hist.iter().enumerate() {
+            if *cnt == 0 { continue }
+            acc += cnt;
+            println!("     len {:>2}: {:>9}  ({:4.1}%)  cum {:4.1}%", len, cnt, *cnt as f64/tot as f64*100.0, acc as f64/tot as f64*100.0);
+        }
+        println!("  nodes with both slots used: {}   total key bytes used per node avg: {:.1} of {}",
+            self.both_slots, self.key_bytes_used as f64 / self.list_nodes.max(1) as f64, crate::line_list_node::KEY_BYTES_CNT);
+        let tn: usize = self.node_klen_hist.iter().sum();
+        let mut cum = 0usize; let mut cum_av = 0usize;
+        println!("  per-NODE total key bytes (cumulative fit):");
+        for (len, cnt) in self.node_klen_hist.iter().enumerate() {
+            cum += cnt; cum_av += self.allval_klen_hist[len];
+            if [6usize,10,14,18,22,26,34,42,50,58,84].contains(&len) {
+                println!("     <= {:>2} bytes: {:5.1}% of all list nodes | {:5.1}% of the {} leaf-only nodes",
+                    len, cum as f64/tn as f64*100.0, cum_av as f64/self.allval_nodes.max(1) as f64*100.0, self.allval_nodes);
+            }
+        }
+        println!("  leaf-only nodes (no child slot): {} of {} ({:4.1}%)", self.allval_nodes, self.list_nodes, self.allval_nodes as f64/self.list_nodes.max(1) as f64*100.0);
+        println!("  nodes at the key cap (key0+key1 >= {}): {}", crate::line_list_node::KEY_BYTES_CNT, self.at_cap);
+    }
+    pub fn report(&self, label: &str, vals: usize) {
+        let t = self.total_bytes() as f64;
+        println!("--- {label} ---");
+        println!("  values                {vals}");
+        println!("  list  nodes {:>9}  bytes {:>11}  ({:4.1}% of trie)", self.list_nodes, self.list_bytes, self.list_bytes as f64/t*100.0);
+        println!("  dense nodes {:>9}  bytes {:>11}  ({:4.1}% of trie)  items {}  avg {:.1}/node",
+            self.dense_nodes, self.dense_bytes, self.dense_bytes as f64/t*100.0, self.dense_items,
+            self.dense_items as f64 / self.dense_nodes.max(1) as f64);
+        println!("  dense slots: len {} cap {} ({:.1}% over-allocated)", self.dense_items, self.dense_cap, (self.dense_cap as f64/self.dense_items.max(1) as f64 - 1.0)*100.0);
+        println!("  cell  nodes {:>9}  bytes {:>11}  ({:4.1}% of trie)  items {}", self.cell_nodes, self.cell_bytes, self.cell_bytes as f64/t*100.0, self.cell_items);
+        println!("  empty (allocated) nodes {}   dangling sentinel slots {}", self.empty_nodes, self.dangling_slots);
+        let vs = self.val_slots + self.dense_val_slots;
+        println!("  values by home: list-node slots {} ({:.2}%)  dense-node slots {} ({:.2}%)",
+            self.val_slots, self.val_slots as f64 / vs.max(1) as f64 * 100.0,
+            self.dense_val_slots, self.dense_val_slots as f64 / vs.max(1) as f64 * 100.0);
+        println!("  TOTAL bytes {:>9}   = {:.1} bytes/value", self.total_bytes(), t / vals.max(1) as f64);
+    }
+}
+
+/// Builds a [MemProfile] for `map`.  See the type docs
+pub fn memory_profile<V: Clone + Send + Sync + Unpin + 'static>(map: &PathMap<V>) -> MemProfile {
+    use crate::trie_node::traverse_physical;
+    use crate::alloc::GlobalAlloc;
+    let cf = core::mem::size_of::<crate::dense_byte_node::OrdinaryCoFree<V, GlobalAlloc>>();
+    let cellcf = core::mem::size_of::<crate::dense_byte_node::CellCoFree<V, GlobalAlloc>>();
+    let list_sz = core::mem::size_of::<crate::line_list_node::LineListNode<V, GlobalAlloc>>();
+    let dense_sz = core::mem::size_of::<crate::dense_byte_node::DenseByteNode<V, GlobalAlloc>>();
+    let cell_sz = core::mem::size_of::<crate::dense_byte_node::CellByteNode<V, GlobalAlloc>>();
+    let Some(root) = map.root() else { return MemProfile::default() };
+    traverse_physical(root, move |node, ctx: MemProfile| {
+        let mut c = ctx;
+        if node.item_count() == 0 { c.empty_nodes += 1; }
+        if let Some(l) = node.as_list() {
+            c.list_nodes += 1; c.list_bytes += list_sz;
+            //A slot whose child link is the empty sentinel is a dangling path: the path exists but
+            //carries no value and leads nowhere
+            if l.is_used_child_0() && unsafe{ l.child_in_slot::<0>() }.is_empty() { c.dangling_slots += 1 }
+            if l.is_used_child_1() && unsafe{ l.child_in_slot::<1>() }.is_empty() { c.dangling_slots += 1 }
+            let (k0, k1) = l.get_both_keys();
+            if c.klen_hist.len() < crate::line_list_node::KEY_BYTES_CNT + 1 { c.klen_hist.resize(crate::line_list_node::KEY_BYTES_CNT + 1, 0); }
+            c.klen_hist[k0.len()] += 1;
+            if k1.len() > 0 { c.klen_hist[k1.len()] += 1; c.both_slots += 1; }
+            c.key_bytes_used += k0.len() + k1.len();
+            if k0.len() + k1.len() >= crate::line_list_node::KEY_BYTES_CNT { c.at_cap += 1; }
+            let ktot = k0.len() + k1.len();
+            if c.node_klen_hist.len() < 2*crate::line_list_node::KEY_BYTES_CNT + 2 { c.node_klen_hist.resize(2*crate::line_list_node::KEY_BYTES_CNT + 2, 0); c.allval_klen_hist.resize(2*crate::line_list_node::KEY_BYTES_CNT + 2, 0); }
+            c.node_klen_hist[ktot] += 1;
+            let has_child = l.is_used_child_0() || l.is_used_child_1();
+            if !has_child { c.allval_nodes += 1; c.allval_klen_hist[ktot] += 1; }
+            if l.is_used_value_0() { c.val_slots += 1 } else if l.is_used_child_0() { c.child_slots += 1 }
+            if l.is_used_value_1() { c.val_slots += 1 } else if l.is_used_child_1() { c.child_slots += 1 }
+        }
+        else if let Some(d) = node.as_dense() {
+            c.dense_nodes += 1; c.dense_items += d.slot_count(); c.dense_val_slots += d.val_slot_count(); c.dense_cap += d.slot_capacity(); c.dense_bytes += dense_sz + d.slot_capacity()*cf;
+        } else if node.tag() == crate::trie_node::CELL_BYTE_NODE_TAG {
+            let n = node.item_count();
+            // each CellCoFree additionally owns a boxed OrdinaryCoFree
+            c.cell_nodes += 1; c.cell_items += n; c.cell_bytes += cell_sz + n*(cellcf + cf);
+        }
+        c
+    }, |a, b| a.merge(b))
+}
+
 #[cfg(test)]
 mod tests {
     use super::{cow_counters, reset_cow_counters};

src/dense_byte_node.rs

@@ -15,6 +15,8 @@ use crate::line_list_node::LineListNode;
 //NOTE: This: `core::array::from_fn(|i| i as u8);` ought to work, but https://github.com/rust-lang/rust/issues/109341
 const ALL_BYTES: [u8; 256] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255];
 
+
+
 /// A ByteNode with insides that **cannot** be shared across threads
 pub type DenseByteNode<V, A> = ByteNode<OrdinaryCoFree<V, A>, A>;
 
@@ -119,6 +121,16 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> ByteNode<Cf, A>
         };
         word_base + (mask_word & preceding_bits).count_ones() as usize
     }
+    /// Number of allocated `CoFree` slots, including unused `Vec` capacity
+    #[inline]
+    pub fn slot_capacity(&self) -> usize { self.values.capacity() }
+    /// Number of slots holding a value
+    #[inline]
+    pub fn val_slot_count(&self) -> usize { self.values.iter().filter(|cf| cf.has_val()).count() }
+    /// Number of occupied `CoFree` slots.  Note this differs from [Self::item_count], which counts
+    /// a slot's child link and value as two separate items
+    #[inline]
+    pub fn slot_count(&self) -> usize { self.values.len() }
     #[inline]
     pub fn reserve_capacity(&mut self, additional: usize) {
         self.values.reserve(additional)
@@ -1828,14 +1840,9 @@ impl<V: Clone + Send + Sync + Lattice, A: Allocator, Cf: CoFree<V=V, A=A>, Other
         let (other_rec, other_val) = other.into_both();
         let rec_status = match self.rec_mut() {
             Some(self_rec) => match other_rec {
-                Some(other_rec) => {
-                    let (status, result) = self_rec.make_mut().join_into_dyn(other_rec);
-                    match result {
-                        Ok(()) => {},
-                        Err(replacement_node) => {*self_rec = replacement_node},
-                    }
-                    status
-                },
+                //NOTE: `TrieNodeODRc::join_into` is the same operation, and additionally short-circuits
+                // when both sides are the same shared subtrie
+                Some(other_rec) => self_rec.join_into(other_rec),
                 None => AlgebraicStatus::Identity,
             },
             None => match other_rec {
@@ -2104,6 +2111,46 @@ impl<V: Clone + Send + Sync + Lattice, A: Allocator, Cf: CoFree<V=V, A=A>, Other
 
         let mut is_identity = self.mask == jm;
 
+        //When `other` brings no new bytes the result mask is `self.mask`, so nothing has to be
+        //inserted and the slots can be joined where they sit.  The general path below allocates a
+        //fresh slot array and moves every slot into it, which for an incremental ingest -- a trie
+        //joined with a batch of paths it mostly already covers -- is the entire cost of the call.
+        if jm == self.mask {
+            let mut is_identity = true;
+            let mut l = 0;
+            let mut r = 0;
+            for i in 0..4 {
+                let mut lm = self.mask.0[i];
+                let om = other.mask.0[i];
+                while lm != 0 {
+                    let index = lm.trailing_zeros();
+                    if (om >> index) & 1 != 0 {
+                        //SAFETY: `other`'s slots are all consumed here, because its mask is a
+                        // subset of `self`'s, and its length is zeroed below so they are not
+                        // dropped a second time
+                        let rv = unsafe { std::ptr::read(other.values.get_unchecked(r)) };
+                        let lv = unsafe { self.values.get_unchecked_mut(l) };
+                        match lv.join_into(rv) {
+                            AlgebraicStatus::Identity => { },
+                            AlgebraicStatus::Element => { is_identity = false; },
+                            AlgebraicStatus::None => unreachable!(), //Some.join(Some) shouldn't create None
+                        }
+                        r += 1;
+                    }
+                    l += 1;
+                    lm ^= 1u64 << index;
+                }
+            }
+            unsafe { other.values.set_len(0) }
+            return if self.mask.is_empty_mask() {
+                AlgebraicStatus::None
+            } else if is_identity {
+                AlgebraicStatus::Identity
+            } else {
+                AlgebraicStatus::Element
+            }
+        }
+
         let jmc = [jm.0[0].count_ones(), jm.0[1].count_ones(), jm.0[2].count_ones(), jm.0[3].count_ones()];
 
         let l = (jmc[0] + jmc[1] + jmc[2] + jmc[3]) as usize;
@@ -2289,28 +2336,33 @@ impl<V: Clone + Send + Sync + Lattice, A: Allocator, Cf: CoFree<V=V, A=A>, Other
 // `other` be differently parameterized types
 impl<V: DistributiveLattice + Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> ByteNode<Cf, A> {
     fn psubtract<OtherCf: CoFree<V=V, A=A>>(&self, other: &ByteNode<OtherCf, A>) -> AlgebraicResult<Self> where Self: Sized {
-        let mut is_identity = true;
-        let mut btn = self.clone();
+        //`btn` stays `None` for as long as the result is still identical to `self`, so a subtraction
+        //that removes nothing from this node -- the common case when the operands barely overlap --
+        //never clones it.  The eager version cloned every node it visited, including its whole slot
+        //array with a refcount bump per slot, and then discarded the lot on the `Identity` return.
+        let mut btn: Option<Self> = None;
 
         for i in 0..4 {
             let mut lm = self.mask.0[i];
             while lm != 0 {
                 let index = lm.trailing_zeros();
 
                 if ((1u64 << index) & other.mask.0[i]) != 0 {
-                    let lv = unsafe { self.get_unchecked(64*(i as u8) + (index as u8)) };
-                    let rv = unsafe { other.get_unchecked(64*(i as u8) + (index as u8)) };
+                    let byte = 64*(i as u8) + (index as u8);
+                    let lv = unsafe { self.get_unchecked(byte) };
+                    let rv = unsafe { other.get_unchecked(byte) };
                     match HeteroDistributiveLattice::psubtract(lv, rv) {
                         AlgebraicResult::None => {
-                            is_identity = false;
-                            btn.remove(64*(i as u8) + (index as u8));
+                            btn.get_or_insert_with(|| self.clone()).remove(byte);
                         },
                         AlgebraicResult::Identity(mask) => {
                             debug_assert_eq!(mask, SELF_IDENT); //subtract is non-commutative
                         },
                         AlgebraicResult::Element(jv) => {
-                            is_identity = false;
-                            let dst = unsafe { btn.get_unchecked_mut(64*(i as u8) + (index as u8)) };
+                            //NOTE: the clone is taken at the first change, so `btn` still matches
+                            //`self` everywhere the loop has already been, and indexing it by byte
+                            //stays correct even after a `remove` has shifted its slots
+                            let dst = unsafe { btn.get_or_insert_with(|| self.clone()).get_unchecked_mut(byte) };
                             *dst = jv;
                         },
                     }
@@ -2320,14 +2372,19 @@ impl<V: DistributiveLattice + Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V,
             }
         }
 
-        if btn.is_empty() {
-            AlgebraicResult::None
-        } else {
-            if is_identity {
+        match btn {
+            //Nothing was removed, so the result is `self` -- and `self` being empty is the only way
+            //an untouched result can be empty
+            None => if self.is_empty() {
+                AlgebraicResult::None
+            } else {
                 AlgebraicResult::Identity(SELF_IDENT)
+            },
+            Some(btn) => if btn.is_empty() {
+                AlgebraicResult::None
             } else {
                 AlgebraicResult::Element(btn)
-            }
+            },
         }
     }
 }

src/lib.rs

@@ -143,6 +143,20 @@ mod bridge_node;
 mod old_cursor;
 
 /// A supertrait that encapsulates the bounds for a value that can be put in a [PathMap]
+///
+/// # Value size, and a cliff at 8 bytes
+///
+/// Most values in a trie live in a list node, whose payload slot is a fixed-size cell shared with
+/// the child pointer. A `V` that fits in that cell is stored inline; a larger one is boxed
+/// individually, one heap allocation per value. The threshold is the size of a node pointer, so
+/// **8 bytes** on 64-bit targets with the default `slim_ptrs` feature.
+///
+/// The cliff is sharp and does not show up in the type system. On a trie of ~92k paths where 99.9%
+/// of values sit in list-node slots, going from an 8-byte value to a 9-byte one costs about 19% on
+/// insertion and 36% on drop, entirely in allocator traffic.
+///
+/// If your value is close to the limit it is worth keeping it under: pack it, shrink an
+/// enum's discriminant, or store an index into a side table rather than the payload itself.
 pub trait TrieValue: Clone + Send + Sync + Unpin {}
 
 impl<T> TrieValue for T where T : Clone + Send + Sync + Unpin {}