Sketch drag: drop the second document copy per frame (#10507)

* claim: Sketch drag: drop the second document copy per frame

* Sketch drag: drop the second document copy per frame

edit_solved copied the document to edit it, then stored it with
`*s.doc_mut() = doc`. The command's undo snapshot still holds the old
document, so doc_mut copied the whole design a second time only to
overwrite it. Store the edited document with Session::set_doc instead,
which forgets the same caches and puts it in a new Arc. The design lands
at a new address as before and the command's end records it for the
crash journal (#7502).

A live drag frame on a design of 200 sketches (20k curves) drops from
19.2 ms to 9.4 ms; 50 sketches: 2.3 ms to 1.7 ms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Markus Kaltenbrunner <kaltenbrunnermarkus@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/sketch.rs

@@ -325,7 +325,7 @@ pub(super) fn edit_solved<T>(
     };
     sketch_store::checked(&sk)?;
     sketch_store::write(&mut doc, id, sk)?;
-    *s.doc_mut() = doc;
+    s.set_doc(doc);
     Ok((out, rep))
 }
 

crates/engine/src/cmd/sketch_drag_live_tests.rs

@@ -126,6 +126,23 @@ fn a_live_frame_solves_once_and_leaves_the_features_after_the_sketch_stale() {
     assert!(s.model.state().bodies.first().expect("a body").body.same(&body), "the body is not rebuilt");
 }
 
+/// A frame copies the design once, to edit it: its undo step holds the design as it was, and a
+/// second copy for storing the edited one would double the work on a large design.
+#[test]
+fn a_live_frame_copies_the_design_once_and_its_undo_step_keeps_the_design_before() {
+    let (mut s, _) = extruded();
+    let before = s.doc.clone();
+    let undo = s.undo.len();
+    crate::tests::doc_copies::take();
+    frames(&mut s, 1, true);
+    assert_eq!(crate::tests::doc_copies::take(), 0, "the edited design is stored, not copied again");
+    assert!(corner(&s).dist(solidcraft_geom::Vec2::new(4.0, 5.0)) < 1e-9);
+    assert_eq!(s.undo.len(), undo + 1);
+    assert!(s.undo.last().is_some_and(|u| std::sync::Arc::ptr_eq(&u.doc, &before)), "undo goes back to the design before");
+    run(&mut s, "edit.undo", json!({}));
+    assert!(std::sync::Arc::ptr_eq(&s.doc, &before));
+}
+
 #[test]
 fn the_drag_end_evaluates_once_to_the_scripted_drags_result() {
     let (mut live, extrude) = extruded();

crates/engine/src/cmd/sketch_perf_tests.rs

@@ -83,3 +83,78 @@ fn sketch_perf() {
         );
     }
 }
+
+/// Fill sketch `id` with `n` constrained rectangles, each with a circle inside.
+fn fill(doc: &mut solidcraft_doc::Document, id: u64, n: usize, x0: f64) {
+    use solidcraft_geom::Vec2;
+    use solidcraft_sketch::ConstraintKind::*;
+    let Ok(sk) = doc.sketch_mut(id) else { return };
+    for k in 0..n {
+        let (x, y) = (x0 + k as f64 * 30.0, 0.0);
+        let p: Vec<usize> = [(0.0, 0.0), (20.0, 0.3), (20.2, 15.0), (-0.1, 15.1)]
+            .iter()
+            .map(|(dx, dy)| sk.add_point(Vec2::new(x + dx, y + dy), None).unwrap())
+            .collect();
+        let l: Vec<usize> = (0..4).map(|i| sk.add_line_pts(p[i], p[(i + 1) % 4], None).unwrap()).collect();
+        sk.add_constraint(Horizontal { l: l[0] }, None).unwrap();
+        sk.add_constraint(Horizontal { l: l[2] }, None).unwrap();
+        sk.add_constraint(Vertical { l: l[1] }, None).unwrap();
+        sk.add_constraint(Vertical { l: l[3] }, None).unwrap();
+        sk.add_constraint(Length { l: l[0], value: 20.0 }, None).unwrap();
+        sk.add_constraint(Length { l: l[1], value: 15.0 }, None).unwrap();
+        sk.add_circle(Vec2::new(x + 10.0, y + 7.5), 3.0, None, None).unwrap();
+    }
+}
+
+/// A large design (`sketches` sketches of `rects` rectangles, `params` parameters), the first
+/// sketch (`first` rectangles) open for editing; per live drag frame: execute time and the
+/// time of one copy of the document.
+#[test]
+#[ignore]
+fn sketch_drag_frame_perf() {
+    let cases: [(usize, usize, usize, usize); 3] = [(50, 10, 200, 2), (200, 20, 500, 2), (200, 20, 500, 50)];
+    for (sketches, rects, params, first) in cases {
+        let mut s = Session::default();
+        for i in 0..params {
+            s.execute("parameters.add", &json!({"name": format!("p{i}"), "expression": format!("{i} mm")})).unwrap();
+        }
+        for k in 0..sketches {
+            s.execute("sketch.create", &json!({"plane": "XY"})).unwrap();
+            let id = s.active_sketch.unwrap();
+            let mut doc = (*s.doc).clone();
+            fill(&mut doc, id, if k == 0 { first } else { rects }, 0.0);
+            *s.doc_mut() = doc;
+            s.execute("sketch.finish", &json!({})).unwrap();
+        }
+        s.execute("sketch.edit", &json!({"sketch": "Sketch1"})).unwrap();
+        let curves: usize = s.doc.features.iter().filter_map(|f| s.doc.sketch(f.id).ok()).map(|k| k.curves.len()).sum();
+        let json_len = serde_json::to_string(&*s.doc).map(|j| j.len()).unwrap_or(0);
+        let frames = 60;
+        let mut frame_ms = Vec::new();
+        for f in 0..frames {
+            let to = [1.0 + f as f64 * 0.05, 1.0 + f as f64 * 0.03];
+            let t = Instant::now();
+            s.execute("sketch.drag", &json!({"entity": "p1", "from": [0, 0], "to": to, "live": true})).unwrap();
+            frame_ms.push(ms(t));
+        }
+        s.execute("sketch.drag_end", &json!({})).ok();
+        let digest = {
+            use std::hash::{Hash, Hasher};
+            let mut h = std::collections::hash_map::DefaultHasher::new();
+            serde_json::to_string(&*s.doc).unwrap_or_default().hash(&mut h);
+            format!("{:016x}", h.finish())
+        };
+        frame_ms.sort_by(f64::total_cmp);
+        let t = Instant::now();
+        for _ in 0..frames {
+            std::hint::black_box((*s.doc).clone());
+        }
+        let copy = ms(t) / frames as f64;
+        let frame = frame_ms[frames / 2];
+        eprintln!(
+            "DRAGPERF sketches={sketches} rects={rects} first={first} params={params} features={} curves={curves} json={json_len}B: frame median {frame:.3} ms, one copy {copy:.3} ms ({:.1} %), design {digest}",
+            s.doc.features.len(),
+            100.0 * copy / frame
+        );
+    }
+}

crates/engine/src/crash_tests.rs

@@ -130,6 +130,29 @@ fn child() {
             assert_eq!(read_live_journal(state).base, json!({"file":path}));
             println!("PREPARED_JOURNAL_OK");
         }
+        // Live sketch drag frames store the edited design without copying it again: each
+        // frame is journaled, and the next frame does not take the design for an outside edit.
+        "sketch_drag" => {
+            s.execute("sketch.create", &json!({"plane": "XY"})).unwrap();
+            s.execute("sketch.rectangle.two_point", &json!({"p0": [5, 5], "p1": [25, 20]})).unwrap();
+            for k in 1..=3 {
+                let (from, to) = ([6.0 - k as f64, 5.0], [5.0 - k as f64, 5.0]);
+                s.execute("sketch.drag", &json!({"entity": "l1.start", "from": from, "to": to, "live": true})).unwrap();
+            }
+            s.execute("sketch.drag_end", &json!({})).unwrap();
+            let journal = read_live_journal(STATE.get().unwrap());
+            assert_eq!(journal.base, json!("new"), "the drag frames continue the replay");
+            // The drag's end only evaluates the model: it changes nothing a replay needs.
+            let ids: Vec<&str> = journal.replay.iter().filter_map(|c| c["command"].as_str()).collect();
+            assert_eq!(ids, ["solid.box", "sketch.create", "sketch.rectangle.two_point", "sketch.drag", "sketch.drag", "sketch.drag"]);
+            let mut replay = Session::default();
+            for command in &journal.replay {
+                replay.execute(command["command"].as_str().unwrap(), &command["params"]).unwrap();
+            }
+            let id = s.active_sketch.unwrap();
+            assert_eq!(replay.doc.sketch(id).ok(), s.doc.sketch(id).ok(), "the replay drags the sketch to the same place");
+            println!("SKETCH_DRAG_JOURNAL_OK");
+        }
         "render" => {
             s.execute("render.settings.set", &json!({"name":"Studio","set":{"exposure_ev":2}})).unwrap();
             s.execute("render.settings.get", &json!({"name":"Studio"})).unwrap();
@@ -212,6 +235,13 @@ fn a_read_only_command_keeps_the_replay_after_body_names_are_pinned() {
     let _ = fs::remove_dir_all(directory);
 }
 
+#[test]
+fn live_sketch_drag_frames_keep_the_replay() {
+    let directory = std::env::temp_dir().join(format!("solidcraft-crash-test-sketch-drag-{}", std::process::id()));
+    let (output, ok) = run_child(&directory, "sketch_drag");
+    assert!(ok && output.contains("SKETCH_DRAG_JOURNAL_OK"), "{output}");
+}
+
 fn only_report(dir: &Path) -> (Summary, Value) {
     let all = list(dir);
     assert_eq!(all.len(), 1, "{all:?}");

crates/engine/src/lib.rs

@@ -932,16 +932,35 @@ impl Session {
 
     /// Mutable document (copy-on-write).
     pub fn doc_mut(&mut self) -> &mut Document {
-        // The placement index keeps the document alive with its answers; let it go before an
-        // in-place edit, so the edit stays in place (a shared handle would clone the document
-        // to a new address behind the crash journal's back, #7502) and no stale lookup outlives it.
+        self.forget_document_caches();
+        #[cfg(test)]
+        if Arc::strong_count(&self.doc) > 1 {
+            tests::doc_copies::record();
+        }
+        Arc::make_mut(&mut self.doc)
+    }
+
+    /// Make `doc` the design, as `*doc_mut() = doc` does, without first copying a shared
+    /// document only to overwrite the copy: a command's undo step still holds the design it
+    /// started from, so `doc_mut` would copy all of it (every feature and sketch) on each
+    /// sketch drag frame. The design lands at a new address either way, and the command's end
+    /// tells the crash journal that address (#7502).
+    pub(crate) fn set_doc(&mut self, doc: Document) {
+        self.forget_document_caches();
+        self.doc = Arc::new(doc);
+    }
+
+    /// Before the document changes: the placement index keeps the document alive with its
+    /// answers; let it go, so an in-place edit stays in place (a shared handle would clone the
+    /// document to a new address behind the crash journal's back, #7502) and no stale lookup
+    /// outlives it.
+    fn forget_document_caches(&mut self) {
         if let Ok(mut index) = self.placement_index_cache.lock() {
             *index = None;
         }
         self.forget_consumed_sketches();
         self.forget_dirty_answer();
         self.model.forget_fingerprints();
-        Arc::make_mut(&mut self.doc)
     }
 
     /// Prepared placements only: optional save caches must not compute a cold world.

crates/engine/src/tests.rs

@@ -6,6 +6,24 @@ use crate::*;
 
 mod mesh_convert;
 
+/// Counts the copies `Session::doc_mut` makes of a shared document.
+pub(crate) mod doc_copies {
+    use std::cell::Cell;
+
+    thread_local! {
+        static COUNT: Cell<usize> = const { Cell::new(0) };
+    }
+
+    pub fn record() {
+        COUNT.with(|c| c.set(c.get() + 1));
+    }
+
+    /// The count since the last call.
+    pub fn take() -> usize {
+        COUNT.with(|c| c.replace(0))
+    }
+}
+
 fn run(s: &mut Session, id: &str, p: Value) -> Value {
     match s.execute(id, &p) {
         Ok(v) => v,