perf(collab): memoise layer depths in diff and skip the sibling scan for a new child (#1961)

* perf(collab): memoise layer depths in diff and skip the sibling scan for a new child

* address review: bound the depth walk by the document's node count

crates/collab/src/diff.rs

@@ -95,7 +95,8 @@ pub fn diff(from: &Replica, to: &Document) -> Result<Diff, Error> {
     let mut changes = fields;
     let new: BTreeSet<NodeId> = added.iter().copied().collect();
     // Creations, parents before children.
-    added.sort_by_cached_key(|id| (depth(to, *id), *id));
+    let mut memo = BTreeMap::new();
+    added.sort_by_cached_key(|id| (depth(to, *id, &mut memo), *id));
     for id in &added {
         if let Some(n) = to.node(*id) {
             changes.push(create_change(n, replica.key(*id))?);
@@ -121,18 +122,30 @@ pub fn diff(from: &Replica, to: &Document) -> Result<Diff, Error> {
     Ok(Diff { changes, replica })
 }
 
-/// Depth of `id` in `doc` (pages are 0).
-fn depth(doc: &Document, id: NodeId) -> usize {
-    let mut d = 0;
+/// Depth of `id` in `doc` (pages are 0). `memo` holds the depths found so far, so a deep chain of
+/// new layers is walked once rather than once per layer.
+fn depth(doc: &Document, id: NodeId, memo: &mut BTreeMap<NodeId, usize>) -> usize {
+    let mut chain = vec![id];
+    let mut base = 0;
     let mut cur = doc.node(id).and_then(|n| n.parent);
     while let Some(p) = cur {
-        d += 1;
-        if d > 100_000 {
+        if let Some(d) = memo.get(&p) {
+            base = *d + 1;
             break;
         }
+        // An acyclic chain is no longer than the document: this only stops a cycle.
+        if chain.len() > doc.node_count() {
+            break;
+        }
+        chain.push(p);
         cur = doc.node(p).and_then(|n| n.parent);
     }
-    d
+    // `chain` runs from `id` up; the last entry's depth is `base` when it ends at a root or a memo hit.
+    let top = chain.len();
+    for (i, c) in chain.iter().enumerate() {
+        memo.insert(*c, base + (top - 1 - i));
+    }
+    memo.get(&id).copied().unwrap_or(0)
 }
 
 /// New keys for the children `kids` (in their new order) whose `old` key no longer fits: the
@@ -196,3 +209,21 @@ fn increasing_run(keys: &[Option<&str>]) -> Vec<bool> {
     }
     kept
 }
+
+#[cfg(test)]
+mod depth_tests {
+    use super::*;
+    use figcraft_doc::NodeKind;
+
+    #[test]
+    fn a_chain_longer_than_a_fixed_bound_is_walked_whole() {
+        let mut doc = Document::new("File");
+        let mut parent = doc.first_page().unwrap();
+        let mut last = parent;
+        for i in 0..100_050 {
+            last = doc.create(parent, NodeKind::Frame, &format!("F{i}"), 0.0, 0.0, 1.0, 1.0).unwrap();
+            parent = last;
+        }
+        assert_eq!(depth(&doc, last, &mut BTreeMap::new()), 100_050);
+    }
+}

crates/collab/src/diff_tests.rs

@@ -310,3 +310,14 @@ fn many_sibling_placements_keep_the_order_of_counting_them() {
         assert_eq!(r.document().node(p).unwrap().children, kids[&p], "children of {p}");
     }
 }
+
+#[test]
+fn a_chain_of_new_nested_layers_is_created_parents_first() {
+    let (a, _, frame, _) = setup();
+    let mut b = a.document().clone();
+    let mut parent = frame;
+    for i in 0..400 {
+        parent = b.create(parent, NodeKind::Frame, &format!("F{i}"), 0.0, 0.0, 10.0, 10.0).unwrap();
+    }
+    assert_eq!(round_trip(&a, &b).iter().filter(|c| matches!(c, Change::Create { .. })).count(), 400);
+}

crates/collab/src/replica.rs

@@ -236,7 +236,9 @@ impl Replica {
                         self.doc.insert_page(node).map_err(doc_err)?;
                     }
                     (false, Some(p)) => {
-                        let at = self.slot(&self.doc.get(*p).map_err(doc_err)?.children, *id, position);
+                        // A new id sits in no sibling list yet: no scan for it, just the binary search.
+                        let siblings = &self.doc.get(*p).map_err(doc_err)?.children;
+                        let at = siblings.partition_point(|c| (self.key(*c), *c) < (position.as_str(), *id));
                         self.doc.insert(node, *p, Some(at)).map_err(doc_err)?;
                     }
                     _ => return Err(Error::Invalid("pages, and only pages, have no parent".into())),