diff --git a/crates/neovm-core/src/buffer/buffer_text/tests/interval_retention.rs b/crates/neovm-core/src/buffer/buffer_text/tests/interval_retention.rs new file mode 100644 index 0000000000..11c945a7dc --- /dev/null +++ b/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}" + ); +} diff --git a/crates/neovm-core/src/buffer/buffer_text/tests/mod.rs b/crates/neovm-core/src/buffer/buffer_text/tests/mod.rs index 6fd0920a37..120dd12bde 100644 --- a/crates/neovm-core/src/buffer/buffer_text/tests/mod.rs +++ b/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 { diff --git a/crates/neovm-core/src/buffer/text_props.rs b/crates/neovm-core/src/buffer/text_props.rs index f566c7ef8c..36ae2ba15a 100644 --- a/crates/neovm-core/src/buffer/text_props.rs +++ b/crates/neovm-core/src/buffer/text_props.rs @@ -644,6 +644,10 @@ impl IntervalRun { struct IntervalTree { root: Option, nodes: Vec, + /// 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, /// 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, seen: &mut FxHashSet) { + 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 { diff --git a/crates/neovm-core/src/buffer/text_props/tests/mod.rs b/crates/neovm-core/src/buffer/text_props/tests/mod.rs index 966bf24163..77fd5f6b0e 100644 --- a/crates/neovm-core/src/buffer/text_props/tests/mod.rs +++ b/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; diff --git a/crates/neovm-core/src/buffer/text_props/tests/retention.rs b/crates/neovm-core/src/buffer/text_props/tests/retention.rs new file mode 100644 index 0000000000..5421231a1d --- /dev/null +++ b/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(¤t); + 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(¤t, 16, name), None); + for table in [¤t, &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)) + ); + } + } +}