fix(textprop): reuse detached interval slots

crates/neovm-core/src/buffer/buffer_text/tests/interval_retention.rs

@@ -0,0 +1,29 @@
+use crate::emacs_core::eval::Context;
+
+#[test]
+fn lisp_property_churn_reuses_interval_slots() {
+    crate::test_utils::init_test_tracing();
+    let mut eval = Context::new();
+    eval.eval_str(
+        "(progn (set-buffer (get-buffer-create \" interval-retention\"))
+                (setq buffer-undo-list t)
+                (insert (make-string 32 ?x))
+                (let ((i 0))
+                  (while (< i 2000)
+                    (set-text-properties 9 25 '(retention-probe 1))
+                    (set-text-properties 1 33 nil)
+                    (setq i (1+ i)))))",
+    )
+    .unwrap();
+    let buffer = eval.buffers.current_buffer().unwrap();
+    let (slots, capacity) = buffer
+        .text
+        .storage
+        .borrow()
+        .text_props
+        .arena_slot_counts_for_test();
+    assert!(
+        slots <= 3 && capacity <= 4,
+        "set-text-properties churn retained {slots} slots, capacity {capacity}"
+    );
+}

crates/neovm-core/src/buffer/buffer_text/tests/mod.rs

@@ -9,6 +9,7 @@ use crate::emacs_core::value::Value;
 
 use super::BufferText;
 
+mod interval_retention;
 mod reverse_scan_test;
 
 fn implemented_kind(kind: BufferTextBackendKind) -> ImplementedBufferTextBackendKind {

crates/neovm-core/src/buffer/text_props.rs

@@ -644,6 +644,10 @@ impl IntervalRun {
 struct IntervalTree {
     root: Option<IntervalId>,
     nodes: Vec<IntervalNode>,
+    /// Head of the detached slots, chained through their `right` links.
+    /// `push_node` reuses them before growing `nodes`, so the arena stays
+    /// bounded by the peak number of simultaneously linked intervals.
+    free_head: Option<IntervalId>,
     /// Positional last-descent memo for `find_id`: `(start, end, id)` of the most
     /// recently located interval, tagged with the tree version it was valid for.
     /// A lookup that lands inside `[start, end)` returns in O(1) instead of
@@ -719,6 +723,7 @@ impl Clone for IntervalTree {
         Self {
             root: self.root,
             nodes: self.nodes.clone(),
+            free_head: self.free_head,
             version: AtomicU64::new(0),
             cache_gen: MemoGeneration::default(),
             cache_start: AtomicUsize::new(0),
@@ -970,11 +975,38 @@ impl IntervalTree {
 
     fn push_node(&mut self, node: IntervalNode) -> IntervalId {
         self.invalidate_find_cache();
+        if let Some(id) = self.free_head {
+            self.free_head = self.nodes[id.0].right;
+            self.nodes[id.0] = node;
+            return id;
+        }
         let id = IntervalId(self.nodes.len());
         self.nodes.push(node);
         id
     }
 
+    /// Return `id` to the free list. The caller must already have removed
+    /// every link to it and must not use `id` afterwards; live ids never move.
+    fn recycle_node(&mut self, id: IntervalId) {
+        self.invalidate_find_cache();
+        let node = &mut self.nodes[id.0];
+        node.left = None;
+        node.parent = None;
+        node.total_length = CharLen::ZERO;
+        node.plist = Value::NIL;
+        node.refresh_cache();
+        node.right = self.free_head;
+        self.free_head = Some(id);
+    }
+
+    /// Drop every interval. `nodes` keeps its capacity for reuse.
+    fn clear(&mut self) {
+        self.invalidate_find_cache();
+        self.root = None;
+        self.nodes.clear();
+        self.free_head = None;
+    }
+
     fn leftmost_id(&self, mut id: IntervalId) -> IntervalId {
         while let Some(left) = self.nodes[id.0].left {
             id = left;
@@ -1078,8 +1110,8 @@ impl IntervalTree {
 
     /// Node `id`, reached through `root` or another node's link, without
     /// the bounds check. Every id a tree holds was minted by `push_node` as
-    /// the index it pushed at, and `nodes` only grows -- `delete_node`
-    /// unlinks a node and leaves its slot in place, and a rebuilt tree comes
+    /// an allocated slot; `recycle_node` reuses only unlinked slots, `clear`
+    /// truncates `nodes` only together with the root, and a rebuilt tree comes
     /// with its own `nodes` -- so a linked id is always in bounds. The
     /// balancing code below is the one user: it reads and rewrites a handful
     /// of neighbouring nodes per step, and the checks were over a quarter of
@@ -1720,6 +1752,29 @@ impl IntervalTree {
         if let Some(root) = self.root {
             walk(self, root, None);
         }
+        // Every arena slot is either linked into the tree or on the free list.
+        fn linked_ids(tree: &IntervalTree, id: Option<IntervalId>, seen: &mut FxHashSet<usize>) {
+            if let Some(id) = id {
+                assert!(seen.insert(id.0), "interval {} linked twice", id.0);
+                linked_ids(tree, tree.nodes[id.0].left, seen);
+                linked_ids(tree, tree.nodes[id.0].right, seen);
+            }
+        }
+        let mut seen = FxHashSet::default();
+        linked_ids(self, self.root, &mut seen);
+        let mut free = self.free_head;
+        while let Some(id) = free {
+            assert!(
+                seen.insert(id.0),
+                "free interval {} is linked or cyclic",
+                id.0
+            );
+            let node = &self.nodes[id.0];
+            assert!(node.left.is_none() && node.parent.is_none());
+            assert!(node.total_length.is_empty() && node.plist.is_nil());
+            free = node.right;
+        }
+        assert_eq!(seen.len(), self.nodes.len(), "unaccounted interval slots");
     }
 
     #[cfg(test)]
@@ -1963,13 +2018,7 @@ impl IntervalTree {
             }
         }
 
-        let node = &mut self.nodes[id.0];
-        node.left = None;
-        node.right = None;
-        node.parent = None;
-        node.total_length = CharLen::ZERO;
-        node.plist = Value::NIL;
-        node.refresh_cache();
+        self.recycle_node(id);
     }
 
     fn interval_deletion_adjustment(
@@ -2029,7 +2078,7 @@ impl IntervalTree {
 
         let mut left_to_delete = end.min(tree_len).saturating_sub(start);
         if left_to_delete == tree_len {
-            self.root = None;
+            self.clear();
             return;
         }
 
@@ -2038,7 +2087,7 @@ impl IntervalTree {
                 return;
             };
             if left_to_delete == self.nodes[root.0].total_length {
-                self.root = None;
+                self.clear();
                 return;
             }
             let deleted = self.interval_deletion_adjustment(root, start, left_to_delete);
@@ -2110,6 +2159,7 @@ impl IntervalTree {
             self.invalidate_find_cache();
         }
 
+        self.recycle_node(id);
         Some(removed_len)
     }
 
@@ -4858,6 +4908,11 @@ impl TextPropertyTable {
         self.intervals.max_depth_for_test()
     }
 
+    #[cfg(test)]
+    pub(crate) fn arena_slot_counts_for_test(&self) -> (usize, usize) {
+        (self.intervals.nodes.len(), self.intervals.nodes.capacity())
+    }
+
     /// `(start, end, plist pairs)` of every interval, empty ones included.
     #[cfg(test)]
     pub(crate) fn interval_plist_runs_for_test(&self) -> Vec<IntervalPlistRun> {

crates/neovm-core/src/buffer/text_props/tests/mod.rs

@@ -1,5 +1,8 @@
 use super::*;
 
+#[cfg(test)]
+mod retention;
+
 #[cfg(test)]
 mod source_slice_graft;
 

crates/neovm-core/src/buffer/text_props/tests/retention.rs

@@ -0,0 +1,147 @@
+use super::*;
+
+/// Set a property on the middle of a fixed 32-character object, then clear
+/// the whole object: every cycle splits and merges intervals without changing
+/// how many are needed at once.
+fn churn(table: &mut TextPropertyTable, cycles: usize) {
+    let name = Value::symbol("retention-probe");
+    for _ in 0..cycles {
+        set_chars_for_object_len(table, 8, 24, 32, vec![(name, Value::fixnum(1))]);
+        set_chars_for_object_len(table, 0, 32, 32, Vec::new());
+    }
+}
+
+#[test]
+fn fixed_size_property_churn_reuses_detached_slots() {
+    crate::test_utils::init_test_tracing();
+    let mut table = TextPropertyTable::new();
+    churn(&mut table, 20_000);
+    assert_eq!(table.intervals.runs().len(), 1);
+    let slots = table.intervals.nodes.len();
+    let capacity = table.intervals.nodes.capacity();
+    assert!(slots <= 3, "fixed-size churn retained {slots} slots");
+    assert!(
+        capacity <= 4,
+        "fixed-size churn retained capacity {capacity}"
+    );
+    table.intervals.assert_invariants_for_test();
+}
+
+#[test]
+fn recycled_slots_keep_cow_snapshots_independent() {
+    use std::rc::Rc;
+    crate::test_utils::init_test_tracing();
+    let name = Value::symbol("retention-probe");
+    let mut current = Rc::new(TextPropertyTable::new());
+    churn(Rc::make_mut(&mut current), 100);
+    // Both sides of the copy-on-write split start with the same free slots
+    // and must reuse them without disturbing each other.
+    let mut other = Rc::clone(&current);
+    Rc::make_mut(&mut current).set_properties_for_object_char_len(
+        char_range(4, 28),
+        char_len(32),
+        vec![(name, Value::fixnum(7))],
+    );
+    let frozen = current.clone();
+    let frozen_runs = frozen.interval_plist_runs_for_test();
+    churn(Rc::make_mut(&mut other), 2_000);
+    churn(Rc::make_mut(&mut current), 2_000);
+    assert_eq!(frozen.interval_plist_runs_for_test(), frozen_runs);
+    assert_eq!(get_at_char(&frozen, 16, name), Some(Value::fixnum(7)));
+    assert_eq!(get_at_char(&other, 16, name), None);
+    assert_eq!(get_at_char(&current, 16, name), None);
+    for table in [&current, &other, &frozen] {
+        table.intervals.assert_invariants_for_test();
+        assert!(table.intervals.nodes.len() <= 5);
+    }
+}
+
+#[test]
+fn trailing_prune_and_whole_delete_reuse_retired_slots() {
+    crate::test_utils::init_test_tracing();
+    let name = Value::symbol("retention-probe");
+    let mut table = TextPropertyTable::new();
+    for _ in 0..5_000 {
+        set_chars(&mut table, 8, 24, vec![(name, Value::fixnum(1))]);
+        // Seed the position and rightmost memos right before retirement.
+        assert!(table.intervals.find_id(char_pos(16)).is_some());
+        clear_chars(&mut table, 0, 32);
+        assert!(table.is_empty());
+        table.intervals.assert_invariants_for_test();
+        assert!(table.intervals.nodes.len() <= 2);
+        set_chars_for_object_len(&mut table, 4, 28, 32, vec![(name, Value::fixnum(2))]);
+        delete_char_range(&mut table, 0, 32);
+        assert_eq!(table.intervals.nodes.len(), 0);
+        assert!(table.intervals.find_id(char_pos(0)).is_none());
+        table.intervals.assert_invariants_for_test();
+    }
+    assert!(table.intervals.nodes.capacity() <= 4);
+}
+
+#[test]
+fn recycling_matches_character_model_under_mixed_edits() {
+    crate::test_utils::init_test_tracing();
+    let name = Value::symbol("syntax-table");
+    let mut table = TextPropertyTable::new();
+    let mut expected = vec![None; 32];
+    let mut seed = 0x81a3_769du64;
+    let mut random = || {
+        seed ^= seed << 13;
+        seed ^= seed >> 7;
+        seed ^= seed << 17;
+        seed as usize
+    };
+    let mut peak = 0;
+    for step in 0..5_000 {
+        let start = random() % expected.len();
+        let end = start + 1 + random() % (expected.len() - start);
+        let value = Value::fixnum((step % 7 + 1) as i64);
+        match random() % 6 {
+            0 => {
+                set_chars_for_object_len(
+                    &mut table,
+                    start,
+                    end,
+                    expected.len(),
+                    vec![(name, value)],
+                );
+                expected[start..end].fill(Some(value));
+            }
+            1 => {
+                put_chars_for_object_len(&mut table, start, end, expected.len(), name, value);
+                expected[start..end].fill(Some(value));
+            }
+            2 => {
+                remove_chars(&mut table, start, end, name);
+                expected[start..end].fill(None);
+            }
+            3 if expected.len() < 64 => {
+                insert_chars_at(&mut table, start, 1);
+                expected.insert(start, None);
+            }
+            4 if expected.len() > 8 => {
+                delete_char_range(&mut table, start, start + 1);
+                expected.remove(start);
+            }
+            _ => {
+                clear_chars(&mut table, start, end);
+                expected[start..end].fill(None);
+            }
+        }
+        table.intervals.assert_invariants_for_test();
+        assert!(table.debug_syntax_caches_consistent().is_ok());
+        peak = peak.max(table.intervals.runs().len());
+        assert!(table.intervals.nodes.len() <= peak + 2);
+        for (pos, value) in expected.iter().enumerate() {
+            assert_eq!(
+                get_at_char(&table, pos, name),
+                *value,
+                "step {step}, pos {pos}"
+            );
+            assert_eq!(
+                table.intervals.find_id(char_pos(pos)),
+                table.intervals.find_id_uncached(char_pos(pos))
+            );
+        }
+    }
+}