#1172: look nodes up by id through a checked cache of their tree paths

A lookup by id walked the tree, so every edit, click and pointer move of a single object cost
time in proportion to the whole document. At 100,000 paths:

- hit test, per click: 1.85 → 0.35 ms
- handle drag, 5,000-anchor path, per move: 6.5 → 2.6 ms
- segment drag, per move: 5.8 → 2.7 ms

Document keeps every node's index path in a cache (id_index, never saved). The tree is edited
through public fields in many places, so the cache isn't kept in step with it; it is checked
instead. A cached path is used only while the node at that path still has the id, a check as deep
as the tree. A path that doesn't check out rebuilds the cache, and an id it doesn't have walks,
as before. A rebuild costs a few walks, so it happens only after a few such misses, which keeps
any edit pattern within a small factor of the walk. Ids that appear more than once are never
cached, so they keep their first place in tree order. index_paths uses the cache for small
batches and one walk for large ones.

The one case that answers differently from a walk: a second node with an existing id put in the
tree after the cache was built, earlier in tree order. The lookup then returns the original node,
also one with that id. The app doesn't make such nodes; damaged files with duplicates are cached
afresh when opened.

cached_lookups_follow_any_edit_of_the_tree checks index_path, node and index_paths against a
walk after inserting at the front, removing, swapping, moving between layers, deleting a layer,
and in a clone sharing the cache. Using cached paths without the check fails it. The doc and
engine suites (2,002 tests) pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmMmc1J8XeNLxhRihWv9cc

crates/doc/src/id_index.rs

@@ -0,0 +1,85 @@
+//! A cache of every node's index path, so a lookup by id checks one path instead of walking the
+//! tree.
+//!
+//! The tree is edited in many places through public fields, so the cache can't be kept in step
+//! with it. It is checked instead: a cached path is used only while the node at that path still
+//! has the id (a check as deep as the tree). A path that no longer checks out rebuilds the cache,
+//! and an id the cache doesn't have (deleted, or added since) is looked up by walking, as before.
+//! An id that appears more than once (a damaged file) is never cached, so lookups keep returning
+//! its first place in tree order.
+//!
+//! The one case a cached lookup can answer differently from a walk: a second node with an
+//! existing id put in the tree *after* the cache was built, earlier in tree order than the first.
+//! The lookup then returns the original node (also one with that id) instead of the new one. The
+//! app never makes such a node (paste and duplicate give new ids, undo restores whole trees);
+//! duplicated ids come from damaged files, which are cached afresh when opened.
+
+use std::collections::HashMap;
+use std::sync::{Arc, RwLock};
+
+use crate::{Node, NodeId};
+
+/// Each id's index path; `None` for an id that appears more than once.
+pub(crate) type Paths = HashMap<NodeId, Option<Vec<usize>>>;
+
+/// Ids found by walking because the cache didn't have them, before the cache is rebuilt: each
+/// rebuild costs a few walks, so rebuilding only after a few keeps a lookup's cost within a small
+/// factor of a walk whatever the edit pattern (say, add an object, look it up, again and again).
+const MISSES_PER_REBUILD: u32 = 4;
+
+/// The cache, shared by clones of a document (each checks every path it uses against its own tree).
+#[derive(Default)]
+pub struct IdIndex(RwLock<Option<Arc<Paths>>>, std::sync::atomic::AtomicU32);
+
+impl IdIndex {
+    /// The current cache, if one was built.
+    pub(crate) fn get(&self) -> Option<Arc<Paths>> {
+        self.0.read().ok().and_then(|g| g.clone())
+    }
+
+    /// Rebuild the cache from `layers` (one walk) and return it.
+    pub(crate) fn rebuild(&self, layers: &[Arc<Node>]) -> Arc<Paths> {
+        fn walk(nodes: &[Arc<Node>], path: &mut Vec<usize>, out: &mut Paths) {
+            for (i, n) in nodes.iter().enumerate() {
+                path.push(i);
+                out.entry(n.id).and_modify(|e| *e = None).or_insert_with(|| Some(path.clone()));
+                if let Some(ch) = n.children() {
+                    walk(ch, path, out);
+                }
+                path.pop();
+            }
+        }
+        let mut out = Paths::new();
+        walk(layers, &mut Vec::new(), &mut out);
+        let out = Arc::new(out);
+        if let Ok(mut g) = self.0.write() {
+            *g = Some(out.clone());
+        }
+        self.1.store(0, std::sync::atomic::Ordering::Relaxed);
+        out
+    }
+
+    /// Note an id found by walking that the cache didn't have; true when it's time to rebuild.
+    pub(crate) fn missed(&self) -> bool {
+        self.1.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + 1 >= MISSES_PER_REBUILD
+    }
+}
+
+impl Clone for IdIndex {
+    fn clone(&self) -> Self {
+        IdIndex(RwLock::new(self.get()), Default::default())
+    }
+}
+
+/// A cache, not document content: two documents are equal whatever their caches hold.
+impl PartialEq for IdIndex {
+    fn eq(&self, _: &Self) -> bool {
+        true
+    }
+}
+
+impl std::fmt::Debug for IdIndex {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("IdIndex")
+    }
+}

crates/doc/src/lib.rs

@@ -13,6 +13,7 @@ pub mod cmyk;
 pub mod corners;
 pub mod graph;
 pub mod hit;
+mod id_index;
 pub mod inks;
 mod inline;
 pub use inline::{SYMBOL_HALF, SYMBOL_SIZES};
@@ -658,6 +659,9 @@ pub struct Document {
     /// state, never saved.
     #[serde(skip)]
     pub puppet: Option<Arc<PuppetPins>>,
+    /// Where each node is in the tree, for lookups by id ([`id_index`]): a cache, never saved.
+    #[serde(skip)]
+    index: id_index::IdIndex,
 }
 
 fn ppi72() -> f64 {
@@ -729,6 +733,7 @@ impl Document {
             print_setup: None,
             assets: vec![],
             puppet: None,
+            index: Default::default(),
         };
         let id = d.alloc_id();
         d.layers.push(Arc::new(Node::layer(id, "Layer 1", LayerColor::Preset(0))));
@@ -758,24 +763,37 @@ impl Document {
 
     /// Find a node anywhere in the tree.
     pub fn node(&self, id: NodeId) -> Option<&Node> {
-        fn find(nodes: &[Arc<Node>], id: NodeId) -> Option<&Node> {
-            for n in nodes {
-                if n.id == id {
-                    return Some(n);
-                }
-                if let Some(ch) = n.children()
-                    && let Some(f) = find(ch, id)
-                {
-                    return Some(f);
-                }
-            }
-            None
-        }
-        find(&self.layers, id)
+        self.index_path(id).and_then(|p| self.node_at(&p))
     }
 
-    /// Index path from the top-level layer list to `id` (e.g. `[0, 3, 1]`).
+    /// Index path from the top-level layer list to `id` (e.g. `[0, 3, 1]`): the first place in
+    /// tree order where `id` appears.
     pub fn index_path(&self, id: NodeId) -> Option<Vec<usize>> {
+        // The cached path, while the node there still has the id ([`id_index`]).
+        let cached = self.index.get();
+        match cached.as_ref().and_then(|m| m.get(&id)) {
+            Some(Some(p)) if self.node_at(p).is_some_and(|n| n.id == id) => return Some(p.clone()),
+            Some(None) => return self.walk_index_path(id),
+            Some(Some(_)) => {
+                // Stale: the tree changed. Rebuild, then answer from it (what a walk gives now).
+                return match self.index.rebuild(&self.layers).get(&id) {
+                    Some(Some(p)) => Some(p.clone()),
+                    Some(None) => self.walk_index_path(id),
+                    None => None,
+                };
+            }
+            None => {}
+        }
+        // Not cached: deleted, added since, or no cache yet.
+        let found = self.walk_index_path(id);
+        if cached.is_none() || (found.is_some() && self.index.missed()) {
+            self.index.rebuild(&self.layers);
+        }
+        found
+    }
+
+    /// [`Document::index_path`] by walking the tree.
+    fn walk_index_path(&self, id: NodeId) -> Option<Vec<usize>> {
         fn find(nodes: &[Arc<Node>], id: NodeId, path: &mut Vec<usize>) -> bool {
             for (i, n) in nodes.iter().enumerate() {
                 path.push(i);
@@ -818,10 +836,23 @@ impl Document {
             }
             false
         }
-        let want: HashSet<NodeId> = ids.into_iter().collect();
+        let mut want: HashSet<NodeId> = ids.into_iter().collect();
         let mut out = HashMap::with_capacity(want.len());
+        // Cached paths that still check out ([`id_index`]); the rest take one walk. For a good
+        // share of the document, one walk is quicker than checking each cached path.
+        if let Some(cache) = self.index.get().filter(|c| want.len().saturating_mul(8) < c.len()) {
+            want.retain(|id| match cache.get(id) {
+                Some(Some(p)) if self.node_at(p).is_some_and(|n| n.id == *id) => {
+                    out.insert(*id, p.clone());
+                    false
+                }
+                _ => true,
+            });
+        }
         if !want.is_empty() {
-            walk(&self.layers, &want, &mut Vec::new(), &mut out);
+            let mut rest = HashMap::with_capacity(want.len());
+            walk(&self.layers, &want, &mut Vec::new(), &mut rest);
+            out.extend(rest);
         }
         out
     }

crates/doc/src/tests_lookups.rs

@@ -78,3 +78,79 @@ fn batched_lookups_match_one_at_a_time() {
         assert_eq!(serde_json::to_string(&a).ok(), serde_json::to_string(&b).ok(), "seed {seed}");
     }
 }
+
+/// What a walk of the tree gives for every id, checked against the cached lookups.
+fn agree(d: &Document, ids: &[NodeId], what: &str) {
+    agree_except(d, ids, None, what);
+}
+
+/// [`agree`], except for `dup`, an id given a second node after the cache was built: lookups of it
+/// may return either of its nodes (see `id_index`), but always one with that id.
+fn agree_except(d: &Document, ids: &[NodeId], dup: Option<NodeId>, what: &str) {
+    if let Some(dup) = dup {
+        assert_eq!(d.node(dup).map(|n| n.id), Some(dup), "{what}: node {dup:?} has its id");
+    }
+    let ids: Vec<NodeId> = ids.iter().copied().filter(|id| Some(*id) != dup).collect();
+    let ids = &ids[..];
+    for id in ids {
+        let walked = d.walk_index_path(*id);
+        assert_eq!(d.index_path(*id), walked, "{what}: index_path {id:?}");
+        let by_walk = walked.as_deref().and_then(|p| d.node_at(p)).map(|n| n as *const Node);
+        assert_eq!(d.node(*id).map(|n| n as *const Node), by_walk, "{what}: node {id:?}");
+    }
+    let batched = d.index_paths(ids.iter().copied());
+    for id in ids {
+        assert_eq!(batched.get(id).cloned(), d.walk_index_path(*id), "{what}: index_paths {id:?}");
+    }
+}
+
+/// The id cache never answers differently from a walk, however the tree is changed behind its
+/// back (through the public fields), including in a clone that shares the cache.
+#[test]
+fn cached_lookups_follow_any_edit_of_the_tree() {
+    for seed in 1..30 {
+        let (mut d, ids) = tree(seed);
+        agree(&d, &ids, "fresh");
+        let snapshot = d.clone(); // shares the cache
+        // Insert at the front of the first layer: every path in it shifts.
+        let extra = d.alloc_id();
+        let leaf = Arc::new(Node::path(extra, shapes::rectangle(Rect::new(0.0, 0.0, 1.0, 1.0)), Appearance::default_art()));
+        if let Some(ch) = Arc::make_mut(&mut d.layers[0]).children_mut() {
+            ch.insert(0, leaf.clone());
+        }
+        let mut all = ids.clone();
+        all.push(extra);
+        agree(&d, &all, "inserted at the front");
+        // Remove a child, swap two, and move one into another layer.
+        if let Some(ch) = Arc::make_mut(&mut d.layers[1]).children_mut()
+            && ch.len() > 3
+        {
+            ch.remove(1);
+            ch.swap(0, 2);
+        }
+        let moved = Arc::make_mut(&mut d.layers[2]).children_mut().and_then(|ch| (!ch.is_empty()).then(|| ch.remove(0)));
+        if let Some(m) = moved
+            && let Some(ch) = Arc::make_mut(&mut d.layers[0]).children_mut()
+        {
+            ch.push(m);
+        }
+        agree(&d, &all, "removed, swapped and moved");
+        // A new duplicate of an existing id, earlier in tree order than the original.
+        let dup = ids[ids.len() / 3];
+        let copy = Arc::new(Node::path(dup, shapes::rectangle(Rect::new(0.0, 0.0, 2.0, 2.0)), Appearance::default_art()));
+        if let Some(ch) = Arc::make_mut(&mut d.layers[0]).children_mut() {
+            ch.insert(0, copy);
+        }
+        agree_except(&d, &all, Some(dup), "a duplicate inserted earlier");
+        // Delete a whole layer, then look everything up again, missing ids included.
+        d.layers.remove(1);
+        agree_except(&d, &all, Some(dup), "a layer deleted");
+        // A rebuilt cache (as after loading a file) knows the duplicate and walks for it.
+        let mut fresh = d.clone();
+        fresh.layers = d.layers.clone();
+        fresh.index = Default::default();
+        agree(&fresh, &all, "a damaged tree, cached afresh");
+        // The clone made before all this still answers for its own tree.
+        agree(&snapshot, &ids, "the earlier clone");
+    }
+}