From 9c5494fd296acfecd0d29b55de4da25d882a220a Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 12:30:25 +0300 Subject: [PATCH 1/2] G5: a layer rendered by two workers at once is shared from the cache (#1812) Preview workers that start together miss the layer cache together, and each kept its own copy of every layer; on the GPU each copy was uploaded and packed into its own Classic 3D atlas, which ran the device out of memory on a cold start. LayerCache::insert now keeps the first buffer of a key and returns it, and the renderer goes on with the returned buffer, so the workers share one. Renders don't wait for each other: on the shared frame pool a blocking wait could end up beneath the work it waits for (tried, it stalled playback for the wait's timeout). Co-Authored-By: Claude Opus 5.5 --- crates/render/src/cache.rs | 53 +++++++++++++++++++++++++++++++------- crates/render/src/lib.rs | 39 +++++++++++++++++----------- crates/render/src/tests.rs | 29 +++++++++++++++++++++ 3 files changed, 96 insertions(+), 25 deletions(-) diff --git a/crates/render/src/cache.rs b/crates/render/src/cache.rs index bae12d1e..26666c3e 100644 --- a/crates/render/src/cache.rs +++ b/crates/render/src/cache.rs @@ -168,15 +168,23 @@ impl LayerCache { // Disk cache: a hit comes back into memory. let (dc, dk) = self.disk_key(key)?; let buf = dc.get_layer(dk)?; - self.insert_mem(key, buf.clone()); - Some(buf) + Some(self.insert_mem(key, buf)) } } } - pub fn insert(&self, key: u64, buf: Arc) { + /// Keep `buf` under `key`, and return the buffer the cache holds for it: the one kept first + /// when another render of the same layer got there before (#1812: preview workers starting + /// together each rendered every layer; sharing the first buffer, the GPU compositor uploads + /// and packs it once instead of once per worker, which ran it out of memory). Renders don't + /// wait for each other: on the frame workers' shared thread pool a wait can end up under + /// the very work it waits for. + pub fn insert(&self, key: u64, buf: Arc) -> Arc { if self.gated() { - return; + return buf; + } + if let Some(kept) = self.inner.lock().ok().and_then(|g| g.map.get(&key).map(|e| e.buf.clone())) { + return kept; } let started = self.inner.lock().ok().and_then(|mut g| g.missed_at.remove(&key)); if let Some((dc, dk)) = self.disk_key(key) { @@ -185,20 +193,24 @@ impl LayerCache { dc.put_layer(dk, &buf); } } - self.insert_mem(key, buf); + self.insert_mem(key, buf) } - fn insert_mem(&self, key: u64, buf: Arc) { + /// See [`LayerCache::insert`]: the buffer kept under `key` (`buf`, unless one was kept first + /// or `buf` is too large to keep). + fn insert_mem(&self, key: u64, buf: Arc) -> Arc { let bytes = buf.img.data.len() * std::mem::size_of::(); - let Ok(mut g) = self.inner.lock() else { return }; + let Ok(mut g) = self.inner.lock() else { return buf }; if bytes > g.budget / 4 { - return; + return buf; } g.clock += 1; let now = g.clock; - if let Some(old) = g.map.insert(key, Entry { buf, bytes, last_use: now }) { - g.bytes -= old.bytes; + if let Some(e) = g.map.get_mut(&key) { + e.last_use = now; + return e.buf.clone(); } + g.map.insert(key, Entry { buf: buf.clone(), bytes, last_use: now }); g.bytes += bytes; if g.bytes > g.budget { // Evict least-recently used entries down to 3/4 of the budget. @@ -214,6 +226,7 @@ impl LayerCache { } } } + buf } /// The memory budget in bytes. @@ -668,3 +681,23 @@ pub fn derive(key: u64, i: u64) -> u64 { i.hash(&mut h); h.finish() } + +#[cfg(test)] +mod shared_insert_tests { + use std::sync::Arc; + + use super::LayerCache; + use effectcraft_effects::Buf; + use effectcraft_raster::Image; + + /// Two renders of one layer kept under one key: both use the buffer kept first. + #[test] + fn the_buffer_kept_first_is_shared() { + let cache = LayerCache::default(); + let buf = || Arc::new(Buf { img: Image::new(4, 4), offset: [0.0; 2], scale: 1.0 }); + let (a, b) = (buf(), buf()); + assert!(Arc::ptr_eq(&cache.insert(3, a.clone()), &a)); + assert!(Arc::ptr_eq(&cache.insert(3, b), &a), "the later render gets the first buffer"); + assert!(Arc::ptr_eq(&cache.get(3).unwrap(), &a)); + } +} diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index eccaed2f..3876cb7a 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -1478,10 +1478,11 @@ impl<'a> Renderer<'a> { let mut nb = (*b).clone(); color::convert(&mut nb.img, &c); let nb = Arc::new(nb); - if let (Some(cache), Some(k)) = (self.cache_to_keep(), key) { - cache.insert(k, nb.clone()); + // The buffer the cache keeps (another worker's, if it kept one first). + match (self.cache_to_keep(), key) { + (Some(cache), Some(k)) => cache.insert(k, nb), + _ => nb, } - nb } fn layer_buf_keyed(&self, ctx: &EvalCtx, layer: &Layer) -> Option<(Arc, Option)> { @@ -1496,9 +1497,11 @@ impl<'a> Renderer<'a> { return Some((b, key)); } let flat = Arc::new(styles::Styled { passes: st.passes.iter().map(|(b, m)| ((**b).clone(), *m)).collect(), body: (*st.body).clone() }.flatten()); - if let (Some(c), Some(k)) = (self.cache_to_keep(), key) { - c.insert(k, flat.clone()); - } + // The buffer the cache keeps (another worker's, if it kept one first). + let flat = match (self.cache_to_keep(), key) { + (Some(c), Some(k)) => c.insert(k, flat), + _ => flat, + }; Some((flat, key)) } @@ -1527,9 +1530,11 @@ impl<'a> Renderer<'a> { } let buf = self.masked_source(ctx, layer)?; let buf = Arc::new(self.apply_effects_timed(ctx, layer, buf, false, usize::MAX, timing.map(|t| &mut t.effects))); - if let (Some(c), Some(k)) = (self.cache_to_keep(), key) { - c.insert(k, buf.clone()); - } + // The buffer the cache keeps (another worker's, if it kept one first). + let buf = match (self.cache_to_keep(), key) { + (Some(c), Some(k)) => c.insert(k, buf), + _ => buf, + }; Some((buf, key)) } @@ -1552,9 +1557,11 @@ impl<'a> Renderer<'a> { buf = self.effect_space_stack(ctx, layer, buf, false, effects, None, origin); } let buf = Arc::new(buf); - if let (Some(c), Some(k)) = (self.cache_to_keep(), key) { - c.insert(k, buf.clone()); - } + // The buffer the cache keeps (another worker's, if it kept one first). + let buf = match (self.cache_to_keep(), key) { + (Some(c), Some(k)) => c.insert(k, buf), + _ => buf, + }; Some(buf) } @@ -2215,9 +2222,11 @@ impl<'a> Renderer<'a> { let mut foot = self.source(ctx, layer)?; let _ = masks::apply(ctx, layer, &mut foot, self.mask_blur(ctx, layer)); let foot = Arc::new(foot); - if let (Some(c), Some(k)) = (self.cache_to_keep(), key) { - c.insert(k, foot.clone()); - } + // The buffer the cache keeps (another worker's, if it kept one first). + let foot = match (self.cache_to_keep(), key) { + (Some(c), Some(k)) => c.insert(k, foot), + _ => foot, + }; Some(foot) } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 72dd2650..f290e31c 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -1010,3 +1010,32 @@ fn a_selected_layer_reading_the_comp_scene_invalidates_its_owner() { let (a, b) = (particles_x(&here), particles_x(&panned)); assert!(b < a - 20.0, "the blended particles follow the camera: {a} -> {b}"); } + +/// #1812: preview workers that render a layer at once end up with one buffer (the one kept +/// first), so the GPU compositor uploads and packs it once, not once per worker. +#[test] +fn workers_rendering_a_layer_at_once_share_one_buffer() { + let (mut p, cid, comp) = setup(); + let mut l = solid(&mut p, &comp, [0.2, 0.5, 0.8], 1600, 1200); + add_effect(&mut p, &mut l, "ec.blur.gaussian", [1600.0, 1200.0], &[("blurriness", Value::Scalar(60.0))]); + p.comp_mut(cid).unwrap().layers.push(l); + let cache = crate::LayerCache::default(); + let start = std::sync::Barrier::new(8); + let bufs: Vec<_> = std::thread::scope(|s| { + let hs: Vec<_> = (0..8) + .map(|_| { + s.spawn(|| { + let mut r = crate::Renderer::new(&p, &crate::NoFootage, crate::RenderOpts::default()); + r.cache = Some(&cache); + let c = p.comp(cid).unwrap(); + let ctx = r.eval_ctx(cid, Tick::ZERO).unwrap(); + start.wait(); + r.layer_buf(&ctx, &c.layers[0]).unwrap() + }) + }) + .collect(); + hs.into_iter().map(|h| h.join().unwrap()).collect() + }); + assert!(bufs.iter().all(|b| std::sync::Arc::ptr_eq(b, &bufs[0])), "one shared buffer"); + assert_eq!(cache.stats().entries, 1); +} From a33011047669688bce56eac117a1d2930496ee1d Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 16:12:22 +0300 Subject: [PATCH 2/2] G5: a styled layer rendered by two workers at once shares its body and passes (#1812) Review: styled() kept the buffers it rendered instead of the ones LayerCache::insert returned, so without a blending-space conversion concurrent misses still returned separate body and pass buffers (and the GPU uploaded each). Co-Authored-By: Claude Opus 5.5 --- crates/render/src/lib.rs | 11 ++++++----- crates/render/src/tests_styles.rs | 32 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 3876cb7a..73b86e6b 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -1589,12 +1589,13 @@ impl<'a> Renderer<'a> { t.cached = false; t.effects.push(("layerStyles".into(), t0.elapsed().as_secs_f64() * 1e3)); } - let body = Arc::new(st.body); - let passes: Vec<_> = st.passes.into_iter().map(|(b, m)| (Arc::new(b), m)).collect(); + let mut body = Arc::new(st.body); + let mut passes: Vec<_> = st.passes.into_iter().map(|(b, m)| (Arc::new(b), m)).collect(); + // The buffers the cache keeps (another worker's, if it kept them first). if let (Some(c), Some(k)) = (self.cache_to_keep(), key) { - c.insert(k, body.clone()); - for (i, (b, _)) in passes.iter().enumerate() { - c.insert(cache::derive(k, i as u64 + 1), b.clone()); + body = c.insert(k, body); + for (i, (b, _)) in passes.iter_mut().enumerate() { + *b = c.insert(cache::derive(k, i as u64 + 1), b.clone()); } } Some(StyledLayer { passes, body, content, key, ckey, plain: false }) diff --git a/crates/render/src/tests_styles.rs b/crates/render/src/tests_styles.rs index 147ff159..b59f009d 100644 --- a/crates/render/src/tests_styles.rs +++ b/crates/render/src/tests_styles.rs @@ -475,3 +475,35 @@ fn gradient_glow_uses_gradient() { let px = img.straight(121, 50); assert!(px[0] > px[2], "near the edge the glow takes the gradient's start colour {px:?}"); } + +/// #1812 (review of #1928): a styled layer rendered by several workers at once shares its body and +/// style passes, as unstyled layers share their buffer. +#[test] +fn workers_styling_a_layer_at_once_share_its_buffers() { + let (p, cid) = scene([1.0; 3], None, |p, c, l| { + add(p, c, l, "dropShadow"); + add(p, c, l, "stroke"); + }); + let cache = crate::LayerCache::default(); + let start = std::sync::Barrier::new(8); + let styled: Vec<_> = std::thread::scope(|s| { + let hs: Vec<_> = (0..8) + .map(|_| { + s.spawn(|| { + let mut r = Renderer::new(&p, &NoFootage, RenderOpts::default()); + r.cache = Some(&cache); + let ctx = r.eval_ctx(cid, Tick::ZERO).unwrap(); + start.wait(); + r.styled_layer(&ctx, &p.comp(cid).unwrap().layers[0]).unwrap() + }) + }) + .collect(); + hs.into_iter().map(|h| h.join().unwrap()).collect() + }); + let first = &styled[0]; + assert!(!first.passes.is_empty(), "the drop shadow is a pass"); + for st in &styled { + assert!(std::sync::Arc::ptr_eq(&st.body, &first.body), "one body"); + assert!(st.passes.iter().zip(&first.passes).all(|(a, b)| std::sync::Arc::ptr_eq(&a.0, &b.0)), "one set of passes"); + } +}