From 7235cf2c5def34521d4f62923e8c5d45e35d6eaa Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 14 May 2026 13:19:10 +0200 Subject: [PATCH] :tada: Reduce heap allocations --- render-wasm/src/main.rs | 9 +++--- render-wasm/src/render.rs | 16 ++++++---- render-wasm/src/state.rs | 3 +- render-wasm/src/state/shapes_pool.rs | 46 +++++++++++++--------------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/render-wasm/src/main.rs b/render-wasm/src/main.rs index 74cc7478fe..c1ebc94839 100644 --- a/render-wasm/src/main.rs +++ b/render-wasm/src/main.rs @@ -121,9 +121,11 @@ pub extern "C" fn render(timestamp: i32) -> Result<()> { // modifier set, so the cost is paid once per rAF rather than // once per pointer move. if get_render_state().options.is_interactive_transform() { - let ids = state.shapes.modifier_ids(); + // Collect into an owned Vec to release the immutable borrow on + // `state.shapes` before the mutable `rebuild_modifier_tiles` call. + let ids = state.shapes.modifier_ids().to_vec(); if !ids.is_empty() { - state.rebuild_modifier_tiles(ids)?; + state.rebuild_modifier_tiles(&ids)?; } } state @@ -856,9 +858,8 @@ pub extern "C" fn set_modifiers() -> Result<()> { with_state!(state, { state.set_modifiers(modifiers); - // TO CHECK if !get_render_state().options.is_interactive_transform() { - state.rebuild_modifier_tiles(ids)?; + state.rebuild_modifier_tiles(&ids)?; } }); Ok(()) diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index 44c5c7e7af..343d15e183 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -2911,10 +2911,14 @@ impl RenderState { // modified shapes (doc-space @ 100% zoom, scale=1.0). This is used as a cheap overlap // guard to decide when cached top-level crops are unsafe to reuse (something is moving // over/inside them), without doing expensive ancestor walks per node. + // + // `modifier_ids` is pre-computed once here and reused throughout the loop to avoid + // repeated allocations (formerly O(N_shapes) HashMap builds) per node. + let modifier_ids = tree.modifier_ids(); let moved_bounds = - if self.options.is_interactive_transform() && !tree.modifier_ids().is_empty() { + if self.options.is_interactive_transform() && !modifier_ids.is_empty() { let mut acc: Option = None; - for id in tree.modifier_ids().iter() { + for id in modifier_ids.iter() { let Some(s) = tree.get(id) else { continue }; let r = self.get_cached_extrect(s, tree, 1.0); acc = Some(match acc { @@ -3007,7 +3011,7 @@ impl RenderState { let use_cached = self.should_use_cached_top_level_during_interactive( node_id, tree, - &tree.modifier_ids(), + modifier_ids, moved_bounds, ); @@ -3722,7 +3726,7 @@ impl RenderState { pub fn rebuild_modifier_tiles( &mut self, tree: ShapesPoolMutRef<'_>, - ids: Vec, + ids: &[Uuid], ) -> Result<()> { // During interactive transform, skip ancestor invalidation: walking up to the // parent frame evicts every tile the frame covers, including dense tiles with @@ -3730,9 +3734,9 @@ impl RenderState { // `ShapesPool::set_modifiers`; the tile index is reconciled post-gesture by // the committing code path (rebuild_touched_tiles). if self.options.is_interactive_transform() { - self.update_tiles_shapes(&ids, tree)?; + self.update_tiles_shapes(ids, tree)?; } else { - let ancestors = all_with_ancestors(&ids, tree, false); + let ancestors = all_with_ancestors(ids, tree, false); self.update_tiles_shapes(&ancestors, tree)?; } Ok(()) diff --git a/render-wasm/src/state.rs b/render-wasm/src/state.rs index 5eed9d66fb..116e7a67c4 100644 --- a/render-wasm/src/state.rs +++ b/render-wasm/src/state.rs @@ -225,8 +225,7 @@ impl State { let _ = get_render_state().render_preview(&self.shapes, timestamp); } - pub fn rebuild_modifier_tiles(&mut self, ids: Vec) -> Result<()> { - // Index-based storage is safe + pub fn rebuild_modifier_tiles(&mut self, ids: &[Uuid]) -> Result<()> { get_render_state().rebuild_modifier_tiles(&mut self.shapes, ids) } diff --git a/render-wasm/src/state/shapes_pool.rs b/render-wasm/src/state/shapes_pool.rs index 9f19dbc50a..d51ce1cabe 100644 --- a/render-wasm/src/state/shapes_pool.rs +++ b/render-wasm/src/state/shapes_pool.rs @@ -49,6 +49,11 @@ pub struct ShapesPoolImpl { modified_shape_cache: HashMap>, /// Transform modifiers, keyed by index modifiers: HashMap, + /// UUIDs of shapes that have an active transform modifier, kept in sync + /// with `modifiers`. Stored explicitly so that `modifier_ids()` is O(K) + /// (K = number of modified shapes) instead of O(N_shapes) — avoids + /// building a full reverse-index HashMap on every call. + modifier_uuids: Vec, /// Structure entries, keyed by index structure: HashMap>, /// Scale content values, keyed by index @@ -69,6 +74,7 @@ impl ShapesPoolImpl { modified_shape_cache: HashMap::default(), modifiers: HashMap::default(), + modifier_uuids: Vec::new(), structure: HashMap::default(), scale_content: HashMap::default(), } @@ -238,7 +244,11 @@ impl ShapesPoolImpl { } self.modifiers = modifiers_with_idx; + // Compute ancestors before consuming `ids` so we can move it into + // `modifier_uuids` without a clone. let all_ids = shapes::all_with_ancestors(&ids, self, true); + // Keep modifier_uuids in sync so modifier_ids() is O(K) not O(N_shapes). + self.modifier_uuids = ids; for uuid in all_ids { if let Some(idx) = self.uuid_to_idx.get(&uuid).copied() { self.modified_shape_cache.insert(idx, OnceCell::new()); @@ -300,19 +310,9 @@ impl ShapesPoolImpl { pub fn clean_all(&mut self) -> Vec { self.clean_shape_cache(); - let modified_uuids: Vec = if self.modifiers.is_empty() { - Vec::new() - } else { - let mut idx_to_uuid: HashMap = - HashMap::with_capacity(self.uuid_to_idx.len()); - for (uuid, idx) in self.uuid_to_idx.iter() { - idx_to_uuid.insert(*idx, *uuid); - } - self.modifiers - .keys() - .filter_map(|idx| idx_to_uuid.get(idx).copied()) - .collect() - }; + // `modifier_uuids` is kept in sync with `modifiers` by `set_modifiers`, + // so we can take it directly — no need to rebuild a reverse index. + let modified_uuids = std::mem::take(&mut self.modifier_uuids); self.modifiers = HashMap::default(); self.structure = HashMap::default(); @@ -325,18 +325,12 @@ impl ShapesPoolImpl { /// Used by the throttled drag path so per-rAF tile invalidation can /// be done once with the current modifier set instead of once per /// pointer move. - pub fn modifier_ids(&self) -> Vec { - if self.modifiers.is_empty() { - return Vec::new(); - } - let mut idx_to_uuid: HashMap = HashMap::with_capacity(self.uuid_to_idx.len()); - for (uuid, idx) in self.uuid_to_idx.iter() { - idx_to_uuid.insert(*idx, *uuid); - } - self.modifiers - .keys() - .filter_map(|idx| idx_to_uuid.get(idx).copied()) - .collect() + /// + /// Returns a reference to avoid allocation on every call — callers + /// inside hot render loops should hold this reference rather than + /// calling `modifier_ids()` repeatedly. + pub fn modifier_ids(&self) -> &[Uuid] { + &self.modifier_uuids } pub fn subtree(&self, id: &Uuid) -> ShapesPoolImpl { @@ -363,6 +357,7 @@ impl ShapesPoolImpl { uuid_to_idx, modified_shape_cache: HashMap::default(), modifiers: HashMap::default(), + modifier_uuids: Vec::new(), structure: HashMap::default(), scale_content: HashMap::default(), } @@ -409,6 +404,7 @@ impl Clone for ShapesPoolImpl { // so it gets lazily rebuilt on demand rather than cloning OnceCell state. modified_shape_cache: HashMap::default(), modifiers: self.modifiers.clone(), + modifier_uuids: self.modifier_uuids.clone(), structure: self.structure.clone(), scale_content: self.scale_content.clone(), }