From df22824fbe844209314f4c872348cd24d44bb47d Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Tue, 13 Oct 2026 12:55:00 +0300 Subject: [PATCH] perf(scene): keep resident wires once, not again in the memo The resident tessellation memo held its own copy of every entity's wires next to the assembled resident set. Runs now stay only in the set: a memo entry whose wires the set holds just marks them valid (InSet) and reads them back through a weak link with the set's layout ranges, so the set remains uniquely owned for in-place patches. A resident build moves new tessellations straight into the set (Pending until it is bound); hidden entities copy their run out before the patch blanks it, and an abandoned patch hands its half-taken set back for the full build that follows. Builds whose runs are recoloured afterwards (reference edit, count) keep owned copies as before. On a drawing with 2.4M nested block lines: wire assembly 3.7s -> 2.3s, memory after it 4.6 GB -> 3.8 GB. Refs #1542, #1698 Co-Authored-By: Claude Opus 5.5 --- src/scene/mod.rs | 286 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 259 insertions(+), 27 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 85a819e1e..c9162f0b0 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -882,12 +882,78 @@ pub(crate) struct ResidentWireLayout { marker_start: usize, } +/// One memoized entity tessellation. +#[derive(Clone)] +pub(crate) enum MemoRun { + /// The wires themselves: an entity with no run in the backing set + /// (hidden, or tessellated after the set was built). + Owned(Arc>), + /// Still valid, and the same wires the backing resident set holds for the + /// entity, so they are kept once instead of twice (#1542). + InSet, + /// Tessellated into the resident set being built, which becomes the + /// backing when it is done; until then the run reads as a miss. + Pending, +} + +/// The resident set `MemoRun::InSet` runs live in. Held weakly so the set +/// stays uniquely owned and can be patched in place; `keep` holds a set an +/// abandoned patch took apart until the following full build has read it. +#[derive(Clone, Default)] +pub(crate) struct MemoBacking { + set: std::sync::Weak>, + ranges: HashMap, + keep: Option>>, +} + +/// A memoized run resolved to its wires. +enum MemoHit { + Owned(Arc>), + Slice(Arc>, usize, usize), +} + +impl MemoHit { + fn wires(&self) -> &[WireModel] { + match self { + MemoHit::Owned(wires) => wires, + MemoHit::Slice(set, start, len) => &set[*start..*start + *len], + } + } +} + +/// Hands the set an abandoned resident patch was taking apart back to the +/// memo, whose `InSet` runs still read from it. +struct ReclaimOnDecline<'a> { + scene: &'a Scene, + wires: Option>, + backed: bool, +} + +impl Drop for ReclaimOnDecline<'_> { + fn drop(&mut self) { + if let Some(wires) = self.wires.take() { + if self.backed { + self.scene.keep_memo_backing(wires); + } + } + } +} + +/// Kill-switch: `OCS_NO_RESIDENT_MEMO` reverts the resident set to the old +/// full re-tessellation on every edit, in case a mutation site is ever found +/// that edits geometry without dropping its handle from the memo. +fn resident_memo_enabled() -> bool { + static EN: std::sync::OnceLock = std::sync::OnceLock::new(); + *EN.get_or_init(|| std::env::var("OCS_NO_RESIDENT_MEMO").is_err()) +} + #[derive(Clone)] pub struct PreparedOpenGeometry { pub wires: Arc>, pub interaction_index: Option>, /// Loader tessellations and their guard; a state mismatch clears the memo. - pub tess_memo: HashMap>>, + pub(crate) tess_memo: HashMap, + pub(crate) tess_memo_backing: MemoBacking, pub tess_guard: u64, /// The loader's submission-order layout for `wires`. /// @@ -1483,6 +1549,7 @@ pub fn prepare_open_geometry( } // Taken, not cloned: the scene is discarded on the next line. let tess_memo = std::mem::take(&mut *scene.resident_tess_memo.borrow_mut()); + let tess_memo_backing = std::mem::take(&mut *scene.resident_memo_backing.borrow_mut()); let tess_guard = scene.resident_tess_guard.get(); let draw_depths = scene.draw_depth_cache.borrow_mut().take(); // The layout the loader already built for exactly these wires. Taken by @@ -1508,6 +1575,7 @@ pub fn prepare_open_geometry( wires, interaction_index, tess_memo, + tess_memo_backing, tess_guard, block_defns, draw_depths, @@ -2538,12 +2606,17 @@ pub struct Scene { /// `tess_memo_guard` (tol / view / anno / offset / bg); a guard mismatch /// (zoom, layout, …) clears it. `bump_geometry` clears it (structural /// change); `mark_entity_dirty` drops one handle (incremental edit). - tess_memo: RefCell>>>, + tess_memo: RefCell>, /// Hash of the tessellation parameters `tess_memo` was built under. When /// the current call's parameters differ, the memo is stale and cleared. tess_memo_guard: std::cell::Cell, /// Camera-independent model tessellation, separate from the culled memo. - resident_tess_memo: RefCell>>>, + resident_tess_memo: RefCell>, + /// The resident set `resident_tess_memo`'s `InSet` runs read from. + resident_memo_backing: RefCell, + /// Set while `resident_wires_for` assembles a set that will become the + /// memo's backing; new runs then go straight into it (`Pending`). + building_resident_set: std::cell::Cell, /// Annotation, background, viewport and glyph-atlas signature. resident_tess_guard: std::cell::Cell, /// Per-mutation delta journal: which handles changed on each `geometry_epoch` @@ -2815,6 +2888,8 @@ impl Scene { tess_memo: RefCell::new(HashMap::default()), tess_memo_guard: std::cell::Cell::new(0), resident_tess_memo: RefCell::new(HashMap::default()), + resident_memo_backing: RefCell::new(MemoBacking::default()), + building_resident_set: std::cell::Cell::new(false), resident_tess_guard: std::cell::Cell::new(0), geometry_deltas: RefCell::new(std::collections::VecDeque::new()), pending_removed_categories: RefCell::new(HashMap::default()), @@ -3005,6 +3080,7 @@ impl Scene { // the entities it touched instead of rebuilding every one of them. if !prepared.tess_memo.is_empty() { *self.resident_tess_memo.borrow_mut() = prepared.tess_memo; + *self.resident_memo_backing.borrow_mut() = prepared.tess_memo_backing; self.resident_tess_guard.set(prepared.tess_guard); } // Reuse the loader's draw-order map with this scene's epoch. @@ -3899,6 +3975,7 @@ impl Scene { // Structural change — drop both per-entity tessellation memos. self.tess_memo.borrow_mut().clear(); self.resident_tess_memo.borrow_mut().clear(); + *self.resident_memo_backing.borrow_mut() = MemoBacking::default(); self.invalidate_dependency_index(); // Can't name the changed handles → force every journal consumer to rebuild. self.push_geometry_delta(epoch, Vec::new(), true); @@ -6902,6 +6979,7 @@ impl Scene { // set is zoom-independent (GPU analytical circles/arcs/ellipses). let t_tess = iced::time::Instant::now(); let mut atlas_gen = crate::scene::text::sdf_atlas::generation(); + self.building_resident_set.set(true); let mut wires = self.wires_for_block_culled( block, None, @@ -6940,6 +7018,7 @@ impl Scene { style_viewport, ); } + self.building_resident_set.set(false); let perf = crate::perf::enabled(); let t_post = perf.then(iced::time::Instant::now); // Synthesized nonprint markers (geo-location daisy) live in model space @@ -6967,6 +7046,16 @@ impl Scene { .set(t_tess.elapsed().as_secs_f32() * 1000.0); self.last_tess_wires.set(wires.len()); let arc = Arc::new(wires); + // The memo keeps one copy of what this set holds; read before the + // previous set is evicted below, which may hold runs it still needs. + if self.uses_resident_memo(block, frozen_layers) { + match &layout { + Some(layout) if self.resident_runs_match_memo(block) => { + self.bind_memo_to_set(&arc, &layout.ranges); + } + _ => self.detach_memo_backing(), + } + } let gen = WIRE_CONTENT_GEN.fetch_add(1, Ordering::Relaxed); self.last_model_wire_gen.set(gen); // A full rebuild of the Model set — the GPU arena must rebuild too, not @@ -7060,6 +7149,98 @@ impl Scene { } } + /// The wires a memoized run stands for; `None` when an `InSet` run's + /// backing set is gone, so the caller tessellates afresh. + fn resolve_memo_run(&self, handle: Handle, run: &MemoRun) -> Option { + match run { + MemoRun::Owned(wires) => Some(MemoHit::Owned(Arc::clone(wires))), + MemoRun::InSet => { + let backing = self.resident_memo_backing.borrow(); + let set = backing.set.upgrade()?; + let &(start, len) = backing.ranges.get(&handle)?; + (start + len <= set.len()).then_some(MemoHit::Slice(set, start, len)) + } + MemoRun::Pending => None, + } + } + + /// Whether building `block` reads and fills the resident memo. + fn uses_resident_memo(&self, block: Handle, frozen_layers: Option<&HashSet>) -> bool { + block == self.model_space_block_handle() + && frozen_layers.is_none_or(|layers| layers.is_empty()) + && resident_memo_enabled() + } + + /// Whether a resident set's entity runs are exactly the memo's: nothing + /// recolours or replaces them after tessellation. + fn resident_runs_match_memo(&self, block: Handle) -> bool { + self.count_display.is_none() + && self.refedit_keep.is_none() + && self.active_live_section(block).is_none() + } + + /// Point the memo at `set`: an entity with a run there keeps no copy of + /// its own; a run only the previous backing held is copied out first. + fn bind_memo_to_set( + &self, + set: &Arc>, + ranges: &HashMap, + ) { + let old = std::mem::take(&mut *self.resident_memo_backing.borrow_mut()); + let old_set = old.set.upgrade(); + self.resident_tess_memo.borrow_mut().retain(|handle, run| { + if ranges.contains_key(handle) { + *run = MemoRun::InSet; + } else if let MemoRun::Pending = run { + return false; + } else if let MemoRun::InSet = run { + match old_set.as_ref().zip(old.ranges.get(handle)) { + Some((wires, &(start, len))) if start + len <= wires.len() => { + *run = MemoRun::Owned(Arc::new(wires[start..start + len].to_vec())); + } + _ => return false, + } + } + true + }); + *self.resident_memo_backing.borrow_mut() = MemoBacking { + set: Arc::downgrade(set), + ranges: ranges.clone(), + keep: None, + }; + } + + /// Give every `InSet` run a copy of its own and drop the backing, before + /// a build whose set will not match the memo. + fn detach_memo_backing(&self) { + let old = std::mem::take(&mut *self.resident_memo_backing.borrow_mut()); + let set = old.set.upgrade(); + self.resident_tess_memo.borrow_mut().retain(|handle, run| { + if let MemoRun::Pending = run { + return false; + } + if let MemoRun::InSet = run { + match set.as_ref().zip(old.ranges.get(handle)) { + Some((wires, &(start, len))) if start + len <= wires.len() => { + *run = MemoRun::Owned(Arc::new(wires[start..start + len].to_vec())); + } + _ => return false, + } + } + true + }); + } + + /// An abandoned patch took the backing set apart: keep it alive for the + /// full build that follows. Runs the patch left untouched are intact, and + /// the runs it blanked were copied out or dropped from the memo first. + fn keep_memo_backing(&self, wires: Vec) { + let keep = Arc::new(wires); + let mut backing = self.resident_memo_backing.borrow_mut(); + backing.set = Arc::downgrade(&keep); + backing.keep = Some(keep); + } + fn resident_wire_layout(wires: &[WireModel]) -> Option { let mut order = Vec::new(); let mut ranges = HashMap::default(); @@ -7144,9 +7325,16 @@ impl Scene { return declined("viewport-style"); } let t_patch = iced::time::Instant::now(); + // A patch whose set will not equal the memo's runs leaves the memo + // holding its own copies, taken while the set is still whole. + let shareable = self.uses_resident_memo(block, frozen_layers) + && self.resident_runs_match_memo(block); + if self.uses_resident_memo(block, frozen_layers) && !shareable { + self.detach_memo_backing(); + } // The entry must exist, be stale, and be uniquely held so we can move // its wires out rather than deep-clone them. - let cached_epoch = { + let (cached_epoch, backed_here) = { let sets = self.resident_wire_sets.borrow(); let Some(entry) = sets.get(&key) else { drop(sets); @@ -7166,7 +7354,13 @@ impl Scene { } return None; } - entry.epoch + let backed_here = shareable + && self + .resident_memo_backing + .borrow() + .set + .ptr_eq(&Arc::downgrade(&entry.wires)); + (entry.epoch, backed_here) }; let Some(deltas) = self.replay_since(cached_epoch) else { return declined("journal-too-old"); @@ -7175,7 +7369,7 @@ impl Scene { // Take ownership of the cached assembly (guaranteed unique above). // `prev_gen` is the content id the GPU currently holds for this set — the // base the wire-arena patch replays from. - let (prev_gen, mut owned, mut layout) = { + let (prev_gen, owned, mut layout) = { let removed = self.resident_wire_sets.borrow_mut().remove(&key)?; ( removed.gen, @@ -7183,6 +7377,14 @@ impl Scene { removed.layout?, ) }; + // Every early return below abandons the patch; the memo's `InSet` + // runs then still need this set, so the guard hands it back. + let mut reclaim = ReclaimOnDecline { + scene: self, + wires: Some(owned), + backed: backed_here, + }; + let owned = reclaim.wires.as_mut()?; if layout.marker_start > owned.len() { return None; } @@ -7227,10 +7429,21 @@ impl Scene { } visible_changed.insert(*h); let memoized = memo_ok - .then(|| self.resident_tess_memo.borrow().get(h).cloned()) + .then(|| { + let memo = self.resident_tess_memo.borrow(); + match memo.get(h)? { + MemoRun::Owned(wires) => Some(wires.as_ref().clone()), + MemoRun::InSet if backed_here => { + let &(start, len) = layout.ranges.get(h)?; + owned.get(start..start + len).map(<[WireModel]>::to_vec) + } + run => self + .resolve_memo_run(*h, run) + .map(|hit| hit.wires().to_vec()), + } + }) .flatten(); - if let Some(raw) = memoized { - let mut faded = raw.as_ref().clone(); + if let Some(mut faded) = memoized { self.apply_refedit_fade(&mut faded, bg); new_runs.insert(*h, faded); continue; @@ -7260,7 +7473,7 @@ impl Scene { { let mut memo = self.resident_tess_memo.borrow_mut(); for (h, a) in memo_updates { - memo.insert(h, a); + memo.insert(h, MemoRun::Owned(a)); } } // Capture whether any replaced/removed OLD run fed the Face3D/fill @@ -7333,6 +7546,18 @@ impl Scene { if start + old_len > layout.marker_start { return None; } + // A hidden entity's memo run lives in these slots: copy it + // out before blanking them, for when it is shown again. + if backed_here && !matches!(kind, ChangeKind::Removed) { + let mut memo = self.resident_tess_memo.borrow_mut(); + if let Some(run) = memo.get_mut(&handle) { + if matches!(run, MemoRun::InSet) { + *run = MemoRun::Owned(Arc::new( + owned[start..start + old_len].to_vec(), + )); + } + } + } for slot in &mut owned[start..start + old_len] { *slot = WireModel::solid(String::new(), Vec::new(), [0.0; 4], false); slot.aabb = [ @@ -7458,7 +7683,7 @@ impl Scene { .is_some_and(|run| run.iter().any(|wire| !wire.fill_tris.is_empty())) }); - let arc = Arc::new(owned); + let arc = Arc::new(reclaim.wires.take().unwrap_or_default()); self.last_tess_wires.set(arc.len()); self.resident_patch_hits .set(self.resident_patch_hits.get() + 1); @@ -7480,6 +7705,9 @@ impl Scene { }), )); } + if shareable { + self.bind_memo_to_set(&arc, &layout.ranges); + } let cur_epoch = self.geometry_epoch; let mut sets = self.resident_wire_sets.borrow_mut(); sets.retain(|_, set| set.epoch == cur_epoch); @@ -9908,9 +10136,12 @@ impl Scene { if changed.contains(&handle) || self.document.get_entity(handle).is_none() { continue; } - if let Some(wire) = memo + let hit = memo .get(&handle) - .and_then(|entity_wires| entity_wires.get(ordinal as usize)) + .and_then(|run| self.resolve_memo_run(handle, run)); + if let Some(wire) = hit + .as_ref() + .and_then(|hit| hit.wires().get(ordinal as usize)) { base_slots.insert((handle_value, ordinal), wires.len() as u32); wires.push(wire.clone()); @@ -11357,13 +11588,6 @@ impl Scene { // bypasses the memo. let frozen_empty = frozen_layers.map(|f| f.is_empty()).unwrap_or(true); let base_ok = is_model_block && frozen_empty; - // Kill-switch: `OCS_NO_RESIDENT_MEMO` reverts the resident set to the old - // full re-tessellation on every edit, in case a mutation site is ever - // found that edits geometry without dropping its handle from the memo. - fn resident_memo_enabled() -> bool { - static EN: std::sync::OnceLock = std::sync::OnceLock::new(); - *EN.get_or_init(|| std::env::var("OCS_NO_RESIDENT_MEMO").is_err()) - } let resident = base_ok && view_aabb.is_none() && resident_memo_enabled(); let memo_active = base_ok && (view_aabb.is_some() || resident); // Reported in one line below. `tess_ms` / `memo_misses` are assigned by @@ -11403,14 +11627,14 @@ impl Scene { } // Classify (serial, cheap): reuse memoized Arcs, collect misses. let t_classify = perf.then(iced::time::Instant::now); - let mut hit_arcs: Vec>> = Vec::new(); + let mut hit_arcs: Vec = Vec::new(); let mut misses: Vec<&EntityType> = Vec::new(); { let memo = memo_cell.borrow(); for e in &visible { let h = e.common().handle; - match memo.get(&h) { - Some(a) => hit_arcs.push(Arc::clone(a)), + match memo.get(&h).and_then(|run| self.resolve_memo_run(h, run)) { + Some(hit) => hit_arcs.push(hit), None => misses.push(*e), } } @@ -11422,7 +11646,7 @@ impl Scene { let t_hits = perf.then(iced::time::Instant::now); let hit_wires: Vec = hit_arcs .par_iter() - .flat_map_iter(|a| a.iter().cloned()) + .flat_map_iter(|hit| hit.wires().iter().cloned()) .collect(); hits_ms = crate::perf::elapsed_ms(t_hits); let t_tess_miss = perf.then(iced::time::Instant::now); @@ -11457,9 +11681,16 @@ impl Scene { // with B-rep vertex snaps before the memo/assembly split. let mut wires = Arc::try_unwrap(a).unwrap_or_else(|a| a.as_ref().clone()); self.attach_solid_snaps(h, &mut wires); - let a = Arc::new(wires); - out.extend(a.iter().cloned()); - memo.insert(h, Arc::clone(&a)); + if resident && self.building_resident_set.get() { + // The resident set built from `out` becomes the + // memo's backing: no second copy (#1542, #1698). + out.extend(wires); + memo.insert(h, MemoRun::Pending); + } else { + let a = Arc::new(wires); + out.extend(a.iter().cloned()); + memo.insert(h, MemoRun::Owned(Arc::clone(&a))); + } } } materialize_ms = crate::perf::elapsed_ms(t_materialize); @@ -13398,6 +13629,7 @@ mod journal_tests { wires: Arc::new(Vec::new()), interaction_index: None, tess_memo: HashMap::default(), + tess_memo_backing: MemoBacking::default(), tess_guard: 0, block_defns: Vec::new(), draw_depths: Some(handed_over),