diff --git a/crates/engine/src/background/remote.rs b/crates/engine/src/background/remote.rs index 6539ac767..27f9396e1 100644 --- a/crates/engine/src/background/remote.rs +++ b/crates/engine/src/background/remote.rs @@ -417,6 +417,10 @@ impl FrameServer { auto = Some((key, gpu)); } } + // A frame still waiting for readbacks is replaced: end it, so every `frame_begin` has its `frame_end`. + if self.pending.take().is_some() { + a.frame_end(); + } a.frame_begin(); } self.pending = Some(Pending { id, project: p, comp, time, opts, t0: web_time::Instant::now(), passes: 0, auto }); @@ -457,6 +461,10 @@ impl FrameServer { { pick.record(key, chose, ms); } + // The frame is done (its `frame_begin` ran in `handle`): the GPU pool ages its sizes. + if let Some(a) = &self.accel { + a.frame_end(); + } vec![( FrameReply::Frame { id: pr.id, width: img.width, height: img.height, ms, passes: pr.passes, gpu, layer_misses, layer_hits }, Some(rgba8_premultiplied(&img)), diff --git a/crates/engine/src/background/remote/tests.rs b/crates/engine/src/background/remote/tests.rs index 85f7a1807..e2db10b8e 100644 --- a/crates/engine/src/background/remote/tests.rs +++ b/crates/engine/src/background/remote/tests.rs @@ -192,6 +192,8 @@ struct Deferred { asked: std::sync::Mutex>, gate: Arc, chains: std::sync::atomic::AtomicUsize, + begins: std::sync::atomic::AtomicUsize, + ends: std::sync::atomic::AtomicUsize, auto: effectcraft_render::AutoPick, } @@ -245,8 +247,12 @@ impl Accelerator for Deferred { Some(&self.auto) } fn frame_begin(&self) { + self.begins.fetch_add(1, std::sync::atomic::Ordering::Relaxed); self.ready.lock().unwrap().clear(); } + fn frame_end(&self) { + self.ends.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + } fn pass_begin(&self) { self.gate.store(false, std::sync::atomic::Ordering::Relaxed); } @@ -316,3 +322,56 @@ fn deferred_readbacks_render_in_passes() { let st = acc.auto.stats(effectcraft_render::AutoKey::new(comp, opts.scale, false)).unwrap(); assert!(st.cpu_frames > 0 && st.gpu_frames > 0, "{st:?}"); } + +/// `frame_begin` and `frame_end` stay paired: early fails (before `frame_begin`), a pending frame replaced by a new render, a finished multi-pass frame, and a CPU frame. +#[test] +fn every_frame_begin_has_a_frame_end() { + use std::sync::atomic::Ordering::Relaxed; + let mut s = Session::default(); + s.execute("file.openDemoProject", json!({})).unwrap(); + let comp = s.active_comp_id().unwrap(); + s.execute("layer.newSolid", json!({"color": "#3388ff", "name": "Blue", "width": 200, "height": 120})).unwrap(); + s.execute("effect.apply", json!({"effect": "ec.blur.gaussian"})).unwrap(); + s.execute("effect.apply", json!({"effect": "ec.stylize.glow"})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#ffcc00", "name": "Yellow", "width": 80, "height": 80})).unwrap(); + s.execute("effect.apply", json!({"effect": "ec.color.levels"})).unwrap(); + let opts = RenderOpts { scale: 0.25, backend: Backend::Gpu, ..Default::default() }; + let t = Tick::from_seconds_f64(1.0); + let mut w = server(&s); + let acc = Arc::new(Deferred::default()); + w.set_accel(Some(acc.clone())); + let mut m = Mirror::default(); + w.handle(round(&m.sync(s.revision, &s.project).unwrap())); + let req = |id, revision, comp, opts: RenderOpts| { + round(&FrameMsg::Render { id, revision, comp, time: t, opts: Box::new(opts), disk: None, layers: false, prefetch: vec![] }) + }; + let load = || (acc.begins.load(Relaxed), acc.ends.load(Relaxed)); + // Early fails never touch the frame clock. + assert!(matches!(w.handle(req(1, s.revision + 1, comp, opts))[0].0, FrameReply::Failed { .. })); + assert!(matches!(w.handle(req(2, s.revision, ItemId(999_999), opts))[0].0, FrameReply::Failed { .. })); + assert_eq!(load(), (0, 0)); + // First GPU pass misses and leaves the frame pending. + assert!(w.handle(req(3, s.revision, comp, opts)).is_empty() && w.waiting()); + assert_eq!(load(), (1, 0)); + // A fail while pending leaves the frame and the counters alone. + assert!(matches!(w.handle(req(4, s.revision + 1, comp, opts))[0].0, FrameReply::Failed { .. })); + assert!(w.waiting()); + assert_eq!(load(), (1, 0)); + // A new valid render replaces the pending frame (ends the old one). + assert!(w.handle(req(5, s.revision, comp, opts)).is_empty() && w.waiting()); + assert_eq!(load(), (2, 1)); + let mut out = vec![]; + for _ in 0..10 { + acc.settle(); + out = w.resume(); + if !w.waiting() { + break; + } + } + assert!(matches!(out[0].0, FrameReply::Frame { id: 5, .. })); + assert_eq!(load(), (2, 2)); + // CPU still pairs begin/end on the accelerator. + let cpu = RenderOpts { backend: Backend::Cpu, ..opts }; + assert!(matches!(w.handle(req(6, s.revision, comp, cpu))[0].0, FrameReply::Frame { id: 6, .. })); + assert_eq!(load(), (3, 3)); +} diff --git a/crates/export/src/lib.rs b/crates/export/src/lib.rs index 560d07978..0a8f6b4ec 100644 --- a/crates/export/src/lib.rs +++ b/crates/export/src/lib.rs @@ -175,7 +175,9 @@ pub fn export(job: &Job, progress: &mut dyn FnMut(&Progress) -> bool) -> Result< pub async fn export_async(job: &Job<'_>, progress: &mut dyn FnMut(&Progress) -> bool) -> Result { let t0 = Instant::now(); let cx = Cx::new(job); + let render = RenderEnd(job.accel); let mut r = export_with(&cx, progress, t0).await; + drop(render); if matches!(r, Err(ExportError::Cancelled)) { cx.remove_cancelled_outputs()?; return r; @@ -188,6 +190,18 @@ pub async fn export_async(job: &Job<'_>, progress: &mut dyn FnMut(&Progress) -> r } +/// Calls [`effectcraft_render::Accelerator::render_end`] when dropped: after the export, and +/// also when it panics or its future is dropped (a browser worker aborting it). +struct RenderEnd<'a>(Option<&'a dyn effectcraft_render::Accelerator>); + +impl Drop for RenderEnd<'_> { + fn drop(&mut self) { + if let Some(a) = self.0 { + a.render_end(); + } + } +} + async fn export_with(cx: &Cx<'_>, progress: &mut dyn FnMut(&Progress) -> bool, t0: Instant) -> Result { let comp = cx.comp().ok_or(ExportError::NoComp)?; let total = cx.settings.frame_count(comp); diff --git a/crates/gpu/src/adv3d.rs b/crates/gpu/src/adv3d.rs index c96e57d2d..b88322316 100644 --- a/crates/gpu/src/adv3d.rs +++ b/crates/gpu/src/adv3d.rs @@ -489,7 +489,7 @@ fn raster_into(e: &mut Enc, s: &Scene) -> Option<(wgpu::Texture, wgpu::Texture)> let zout = tex(DEPTH_OUT, rt); let depth = tex(DEPTH, wgpu::TextureUsages::RENDER_ATTACHMENT); let (cv, zv, dv) = (color.create_view(&Default::default()), zout.create_view(&Default::default()), depth.create_view(&Default::default())); - let enc = e.encoder(); + let enc = e.record(&[]); { let mut pass = enc.begin_render_pass(&wgpu::RenderPassDescriptor { label: Some("adv3d"), @@ -763,7 +763,7 @@ fn dispatch(e: &mut Enc, kernel: &str, u0: [u32; 4], f0: [f32; 4], b: Binds, gro } entries.push(wgpu::BindGroupEntry { binding: 10, resource: wgpu::BindingResource::TextureView(b.out.unwrap_or(&post.dummy_out)) }); let bg = g.device.create_bind_group(&wgpu::BindGroupDescriptor { label: Some(kernel), layout: &post.bgl, entries: &entries }); - let enc = e.encoder(); + let enc = e.record(&[]); let mut pass = enc.begin_compute_pass(&wgpu::ComputePassDescriptor { label: Some(kernel), timestamp_writes: None }); pass.set_pipeline(pipe); pass.set_bind_group(0, &bg, &[]); @@ -799,7 +799,7 @@ fn read_buffer(e: &mut Enc, buf: &wgpu::Buffer, size: u64) -> Option> { usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST, mapped_at_creation: false, }); - e.encoder().copy_buffer_to_buffer(buf, 0, &rb, 0, size); + e.record(&[]).copy_buffer_to_buffer(buf, 0, &rb, 0, size); e.submit(); let out = e.g.read_buffer(&rb, row, row, 1).map_err(|e| log::error!("gpu 3d readback: {e}")).ok()?; Some(out) @@ -922,6 +922,7 @@ fn depth_of_field(e: &mut Enc, acc: &wgpu::Buffer, depth: &wgpu::Buffer, (w, h): /// The resolved run as an image in the canvas's encoding, composited over `canvas` when given. pub(crate) fn finish(e: &mut Enc, r: &Resolved, encode: bool, canvas: Option<&GpuImage>) -> GpuImage { + canvas.inspect(|c| e.uses(c)); let out = e.image(r.width, r.height); let ov = out.texture.create_view(&Default::default()); let cv = canvas.map(|c| c.texture.create_view(&Default::default())); @@ -1023,7 +1024,7 @@ fn render_prepared_deferred(g: &GpuContext, d: &std::sync::Arc GpuImage { + e.uses(canvas); let out = e.image(canvas.width, canvas.height); e.copy_into(canvas, &out, 0, 0); if pixels.is_empty() { diff --git a/crates/gpu/src/context.rs b/crates/gpu/src/context.rs index 44e777b5a..2c9a40735 100644 --- a/crates/gpu/src/context.rs +++ b/crates/gpu/src/context.rs @@ -42,7 +42,17 @@ pub const FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba32Float; const UPLOAD_BUDGET: usize = 1536 << 20; /// Working textures kept for reuse once their last image is dropped (bytes). -const POOL_BUDGET: usize = 768 << 20; +const POOL_BUDGET: usize = 256 << 20; + +/// Frames after which a size the pool was not asked for is stale: only stale sizes are +/// evicted to make room (see [`Pool::make_free`]). +/// +/// The clock is coarse: it advances on every finished frame ([`GpuContext::pool_frame_end`]) +/// from any thread, so with many frames in flight (an export renders up to 16 at once) this +/// means "not asked for during the last 2 completions", not "during 2 frames' rendering". A +/// size a slower in-flight frame still needs can go stale and be evicted. That costs a pool +/// miss (a new allocation), never a wrong pixel. +const STALE_FRAMES: u64 = 2; /// Readback staging buffers kept for reuse across sizes (bytes). const STAGING_BUDGET: u64 = 256 << 20; @@ -66,6 +76,20 @@ impl GpuImage { pub fn new(texture: wgpu::Texture, width: u32, height: u32) -> GpuImage { GpuImage { texture, width, height, _lease: None, levels: None } } + + /// This image leaves the [`Enc`] that made it (read by an encoder that may stay open, see + /// [`Enc::uses`]): when its last clone drops, its texture waits for every open encoder + /// ([`Pool::release`]) instead of going back to that `Enc` for immediate reuse. + pub(crate) fn escape(&self) { + if let Some(l) = &self._lease { + l.escaped.store(true, std::sync::atomic::Ordering::Relaxed); + } + } + + /// A pooled image of another `Enc` than `owner` (see [`GpuImage::escape`]). + fn foreign_to(&self, owner: u64) -> bool { + self._lease.as_ref().is_some_and(|l| l.owner != owner) + } } /// Hands a pooled texture back to its [`Pool`] when the last [`GpuImage`] holding it drops. @@ -74,6 +98,10 @@ struct Lease { pool: Weak>, /// The [`Enc`] that took it (see [`Pool::owned`]). owner: u64, + /// Another [`Enc`] recorded work on it ([`Enc::uses`], [`GpuImage::escape`]): + /// its owner can no longer reuse it at once, since that other encoder may still be open, so + /// it is released through the guarded [`Pool::release`]. + escaped: std::sync::atomic::AtomicBool, /// The pool's [`Pool::generation`] when it was taken. generation: u64, } @@ -89,7 +117,12 @@ impl Drop for Lease { if let Some(pool) = self.pool.upgrade() && let Ok(mut p) = pool.lock() { - p.release_owned(self.texture.clone(), self.owner, self.generation); + if !self.escaped.load(std::sync::atomic::Ordering::Relaxed) { + p.release_owned(self.texture.clone(), self.owner, self.generation); + } else if self.generation == p.generation { + // Like `release_owned`, nothing taken before a failed frame comes back (#1319). + p.release(self.texture.clone()); + } } } } @@ -103,7 +136,12 @@ impl Drop for Lease { /// it was released has been submitted (or dropped). #[derive(Default)] pub(crate) struct Pool { - free: HashMap<(u32, u32), Vec>, + /// Free textures by size, each with the [`Pool::stamp`] at which it was freed (oldest first). + free: HashMap<(u32, u32), std::collections::VecDeque<(wgpu::Texture, u64)>>, + /// Counts frees, so the free list evicts the oldest texture first. + stamp: u64, + /// Bytes the free list may hold (0 = [`POOL_BUDGET`]); tests lower it per context. + budget: usize, /// (texture, newest encoder open at its release). released: Vec<(wgpu::Texture, u64)>, /// Encoders with recorded, unsubmitted work. @@ -113,6 +151,25 @@ pub(crate) struct Pool { /// Working textures reused / newly allocated (diagnostics, tests). hits: u64, misses: u64, + /// Free textures dropped to make room for newer ones (diagnostics, tests). + evicted: u64, + /// Output frames done so far ([`GpuContext::pool_frame_end`]). + frame: u64, + /// Per size, the [`Pool::frame`] in which it was last asked for (hit or miss). + asked: HashMap<(u32, u32), u64>, + /// Bytes of evicted textures whose GPU memory may not be freed yet: wgpu frees a dropped + /// texture only once the submissions that used it are done. Counted against the budget + /// for evicting, so evicting and reallocating cannot stack up freed-but-held memory. + dying: Arc, + /// The [`Pool::dying`] bytes left after a [`GpuContext::pool_frame_end`] wait that cleared + /// none of them (`0` = not stalled). While `dying` does not drop below it, later frames only + /// poll instead of waiting again (each wait could take up to [`crate::readback::TIMEOUT`] + /// when the queue stopped completing work). + stalled: usize, + /// Number of empty-submission waits from [`GpuContext::pool_frame_end`] (diagnostics, tests). + waits: u64, + /// Reports when the submissions that may use an evicted texture are done. + queue: Option, /// Textures released while the [`Enc`] that took them is alive, kept for that `Enc` only. /// Its later commands run after everything it recorded or submitted before (one queue, /// recording order), so it can overwrite them at once instead of allocating a new texture @@ -234,13 +291,100 @@ impl Pool { } } + fn budget(&self) -> usize { + if self.budget == 0 { POOL_BUDGET } else { self.budget } + } + + /// Keep `t` on the free list if it fits. Otherwise make room by evicting stale sizes, least + /// recently freed first: sizes not asked for in the last [`STALE_FRAMES`] output frames (a + /// blur bound or padded layer size that no longer occurs), so they age out instead of + /// filling the budget for good. Sizes still in use are never evicted for a newcomer: a + /// frame that frees more than the budget in a repeating order would otherwise evict, on + /// every free, the texture the next call takes first (LRU on a cycle), and miss on every + /// take. If nothing stale makes room, `t` is dropped (the kept set keeps hitting). + /// + /// Evicted textures count against the budget ([`Pool::dying`]) until the GPU work + /// submitted so far is done. That throttles eviction to roughly one budget ahead of the + /// GPU; it is not a hard bound (wgpu frees a dropped texture only when the device is + /// maintained, which can be later than the work-done callback). fn make_free(&mut self, t: wgpu::Texture) { + use std::sync::atomic::Ordering::Relaxed; let b = tex_bytes(&t); - if self.bytes + b > POOL_BUDGET { + let budget = self.budget(); + if b > budget { + return; + } + let mut evicted = 0; + while self.bytes + b > budget { + let frame = self.frame; + let asked = &self.asked; + let stale = |k: &(u32, u32)| asked.get(k).is_none_or(|f| frame.saturating_sub(*f) >= STALE_FRAMES); + let Some(oldest) = self.free.iter().filter(|(k, _)| stale(k)).filter_map(|(k, q)| q.front().map(|(_, s)| (*s, *k))).min().map(|(_, k)| k) else { + break; + }; + let ob = oldest.0 as usize * oldest.1 as usize * 16; + if self.dying.load(Relaxed) + evicted + ob > budget { + break; + } + let Some(q) = self.free.get_mut(&oldest) else { break }; + if let Some((old, _)) = q.pop_front() { + self.bytes -= tex_bytes(&old); + evicted += ob; + self.evicted += 1; + } + if q.is_empty() { + self.free.remove(&oldest); + } + } + if evicted > 0 { + self.dying.fetch_add(evicted, Relaxed); + let dying = self.dying.clone(); + match &self.queue { + Some(q) => q.on_submitted_work_done(move || { + let _ = dying.try_update(Relaxed, Relaxed, |d| Some(d.saturating_sub(evicted))); + }), + None => { + let _ = dying.try_update(Relaxed, Relaxed, |d| Some(d.saturating_sub(evicted))); + } + } + } + if self.bytes + b > budget { return; } self.bytes += b; - self.free.entry((t.width(), t.height())).or_default().push(t); + self.stamp += 1; + self.free.entry((t.width(), t.height())).or_default().push_back((t, self.stamp)); + } + + /// An output frame is done: sizes not asked for since age towards [`STALE_FRAMES`]. + /// Returns the evicted bytes the caller should wait for: 0 when none still count, or when a + /// previous wait cleared none and none has been freed since. + fn frame_end(&mut self) -> usize { + self.frame += 1; + let (frame, free) = (self.frame, &self.free); + self.asked.retain(|k, f| frame - *f < STALE_FRAMES || free.contains_key(k)); + let d = self.dying.load(std::sync::atomic::Ordering::Relaxed); + if d == 0 { + self.stalled = 0; + return 0; + } + if self.stalled != 0 && d >= self.stalled { + // No progress since the stalled wait: do not wait again. + return 0; + } + self.stalled = 0; + d + } + + /// A [`GpuContext::pool_frame_end`] wait finished with `before` dying bytes still counted + /// beforehand. A wait that freed nothing (timed out, or callbacks not run synchronously, + /// e.g. on WebGPU where poll does not block) marks the pool stalled. + fn waited(&mut self, before: usize) { + self.waits += 1; + let d = self.dying.load(std::sync::atomic::Ordering::Relaxed); + if d >= before { + self.stalled = d; + } } fn open(&mut self) -> u64 { @@ -291,14 +435,54 @@ impl Pool { self.release(t); } + /// An owner for [`Enc::lent`]. It is a fresh id each time, so a lease of an earlier lent + /// `Enc` that drops later (an image that outlived its call) finds no owner and goes + /// through [`Pool::release`], which waits for every open encoder. + fn lend(&mut self) -> u64 { + self.register() + } + + /// [`Enc::lent`] is done with `owner` (its work submitted or discarded): its kept textures + /// go to the shared free list at once. The free list has one [`POOL_BUDGET`] (only stale + /// sizes are evicted), so finished calls no longer keep a budget each (16 frame threads + /// kept up to 16 × `POOL_BUDGET`). The budget caps the free list only: textures waiting + /// for open encoders ([`Pool::released`]) and those held by live `Enc`s are outside it. + /// + /// They skip [`Pool::release`]'s wait for open encoders: a kept texture's lease has + /// dropped and only this `Enc` recorded work on it, which is already submitted or + /// discarded, so later work cannot overtake it on the queue. That is enforced, not assumed: + /// a lease read by another `Enc` is marked escaped + /// ([`GpuImage::escape`]) and released through [`Pool::release`] whenever it drops, and a + /// lease that drops after this call finds no owner and goes the same way. + fn give_back(&mut self, owner: u64) { + if let Some(m) = self.owned.remove(&owner) { + for t in m.into_values().flatten() { + self.make_free(t); + } + } + } + + /// Bytes of working textures kept for reuse: the free list, those waiting for open + /// encoders and those kept by live `Enc`s. + fn retained(&self) -> usize { + let owned = self.owned.values().flat_map(|m| m.values().flatten()); + self.released.iter().map(|(t, _)| t).chain(owned).map(tex_bytes).fold(self.bytes, usize::saturating_add) + } + fn take_owned(&mut self, owner: u64, w: u32, h: u32) -> Option { let t = self.owned.get_mut(&owner)?.get_mut(&(w, h))?.pop()?; + self.asked.insert((w, h), self.frame); self.hits += 1; Some(t) } fn take(&mut self, w: u32, h: u32) -> Option { - let t = self.free.get_mut(&(w, h)).and_then(Vec::pop); + self.asked.insert((w, h), self.frame); + let q = self.free.get_mut(&(w, h)); + let t = q.and_then(|q| q.pop_back()).map(|(t, _)| t); + if self.free.get(&(w, h)).is_some_and(|q| q.is_empty()) { + self.free.remove(&(w, h)); + } match &t { Some(t) => { self.bytes -= tex_bytes(t); @@ -739,6 +923,7 @@ impl GpuContext { init_resource(&device, "dummy storage buffer", || buffer_init(&device, &queue, Some("dummy"), &[0u8; 16], wgpu::BufferUsages::STORAGE))?; let dummy_tex = init_resource(&device, "dummy texture view", || dummy.create_view(&Default::default()))?; let max_dim = device.limits().max_texture_dimension_2d; + let pool = Arc::new(Mutex::new(Pool { queue: Some(queue.clone()), ..Default::default() })); Ok(GpuContext { device, queue, @@ -758,7 +943,7 @@ impl GpuContext { uploads: Mutex::new(Uploads::default()), upload_budget: std::sync::atomic::AtomicUsize::new(UPLOAD_BUDGET), upload_full_warned: Default::default(), - pool: Default::default(), + pool, staging: Default::default(), transfers: Default::default(), zeros: Default::default(), @@ -814,6 +999,23 @@ impl GpuContext { }) } + /// Waits on `submission_index` up to `timeout` so wgpu frees resources dropped before that + /// submission. A timeout simply moves on without retiring the context or its readbacks; any + /// other poll error retires the readbacks through the failure path. + fn poll_wait(&self, submission_index: wgpu::SubmissionIndex, timeout: std::time::Duration) { + if self.readbacks.check().is_err() { + return; + } + match self.device.poll(wgpu::PollType::Wait { submission_index: Some(submission_index), timeout: Some(timeout) }) { + Ok(_) | Err(wgpu::PollError::Timeout) => {} + Err(e) => { + let error = format!("GPU poll wait: {e}"); + self.readbacks.retire(error.clone()); + log::error!("{error}"); + } + } + } + /// Start a bounded buffer mapping. Retired contexts reject new requests immediately. pub(crate) fn map_readback( &self, @@ -1054,7 +1256,7 @@ impl GpuContext { let (reused, generation) = self.pool.lock().map(|mut p| (p.take_owned(owner, w, h).or_else(|| p.take(w, h)), p.generation)).unwrap_or((None, u64::MAX)); let was_reused = reused.is_some(); let texture = reused.unwrap_or_else(|| self.texture(w, h)); - let lease = Arc::new(Lease { texture: texture.clone(), pool: Arc::downgrade(&self.pool), owner, generation }); + let lease = Arc::new(Lease { texture: texture.clone(), pool: Arc::downgrade(&self.pool), owner, escaped: Default::default(), generation }); (GpuImage { texture, width: w, height: h, _lease: Some(lease), levels: None }, was_reused) } @@ -1081,6 +1283,73 @@ impl GpuContext { } } + /// Bytes of working textures the pool keeps for reuse (see [`GpuContext::trim_pool`]). + pub fn pool_retained_bytes(&self) -> usize { + self.pool.lock().map(|p| p.retained()).unwrap_or_default() + } + + /// Drop the working textures kept for reuse (a render is done). Textures still waiting for + /// an open encoder and those of live `Enc`s are unaffected. + pub fn trim_pool(&self) { + if let Ok(mut p) = self.pool.lock() { + p.clear(); + } + } + + /// An output frame is done: the pool ages the sizes not asked for since (only sizes unused + /// for [`STALE_FRAMES`] frames are evicted to make room for others). When evicted textures + /// still count ([`Pool::dying`]), waits once so their work-done callbacks can clear that + /// count; if the wait frees none of them, later frames only poll until `dying` drops (a + /// stopped queue must not re-wait [`crate::readback::TIMEOUT`] on every frame). + pub fn pool_frame_end(&self) { + let before = self.pool.lock().map(|mut p| p.frame_end()).unwrap_or_default(); + if before > 0 { + // A work-done callback waits for a submission; an empty one closes the wait. + let index = self.queue.submit([]); + self.poll_wait(index, crate::readback::TIMEOUT); + if let Ok(mut p) = self.pool.lock() { + p.waited(before); + } + } else { + let _ = self.poll_readbacks(wgpu::PollType::Poll); + } + } + + /// Bytes the pool's free list may keep (tests lower it on a context of their own). + #[cfg(test)] + pub(crate) fn set_pool_budget(&self, bytes: usize) { + if let Ok(mut p) = self.pool.lock() { + p.budget = bytes; + } + } + + /// Free textures evicted to make room for newer ones. + #[cfg(test)] + pub(crate) fn pool_evictions(&self) -> u64 { + self.pool.lock().map(|p| p.evicted).unwrap_or_default() + } + + /// Empty-submission waits from [`GpuContext::pool_frame_end`]. + #[cfg(test)] + pub(crate) fn pool_waits(&self) -> u64 { + self.pool.lock().map(|p| p.waits).unwrap_or_default() + } + + /// Add or subtract phantom [`Pool::dying`] bytes without a work-done callback (tests that + /// simulate a queue which stopped completing work). + #[cfg(test)] + pub(crate) fn pool_charge_dying(&self, bytes: isize) { + use std::sync::atomic::Ordering::Relaxed; + if let Ok(p) = self.pool.lock() { + if bytes >= 0 { + p.dying.fetch_add(bytes as usize, Relaxed); + } else { + let sub = (-bytes) as usize; + let _ = p.dying.try_update(Relaxed, Relaxed, |d| Some(d.saturating_sub(sub))); + } + } + } + /// Texture pool counters: (working textures reused, newly allocated). pub fn pool_stats(&self) -> (u64, u64) { self.pool.lock().map(|p| (p.hits, p.misses)).unwrap_or_default() @@ -1197,6 +1466,8 @@ pub(crate) struct Enc<'g> { missing: bool, /// This `Enc`'s id in the texture pool (textures it may reuse at once). owner: u64, + /// `owner` is lent ([`Enc::lent`]) and outlives this `Enc`. + lent: bool, } /// Images bound to a compute kernel's source, optional auxiliary and output slots. @@ -1213,7 +1484,11 @@ impl Drop for Enc<'_> { if let Some((_, id)) = self.enc.take() { p.close(id); } - p.unregister(self.owner); + if self.lent { + p.give_back(self.owner); + } else { + p.unregister(self.owner); + } } } } @@ -1221,7 +1496,27 @@ impl Drop for Enc<'_> { impl<'g> Enc<'g> { pub fn new(g: &'g GpuContext) -> Enc<'g> { let owner = g.pool.lock().map(|mut p| p.register()).unwrap_or(0); - Enc { g, enc: None, pending: 0, missing: false, owner } + Enc { g, enc: None, pending: 0, missing: false, owner, lent: false } + } + + /// An `Enc` for self-contained work (an effect chain): once it is dropped, its working + /// textures can be reused at once ([`Pool::give_back`]). Its images stay with it, except a + /// result another `Enc` records work on ([`Enc::uses`]), whose lease then releases through + /// the guarded [`Pool::release`]. A plain [`Enc::new`] hands its + /// textures back through the shared pool, where they wait for every open encoder, and a + /// GPU frame keeps its encoder open while the CPU renders layers with GPU effects, so every + /// effect's intermediates stayed allocated until the frame ended. + pub(crate) fn lent(g: &'g GpuContext) -> Enc<'g> { + let owner = g.pool.lock().map(|mut p| p.lend()).unwrap_or(0); + Enc { g, enc: None, pending: 0, missing: false, owner, lent: true } + } + + /// `img` is used by this `Enc`: if another `Enc` made it, that one must not reuse it at once + /// (this encoder may still be open when its lease drops), so mark it escaped. + pub(crate) fn uses(&self, img: &GpuImage) { + if img.foreign_to(self.owner) { + img.escape(); + } } /// Whether a dispatch since the last call named a kernel the device doesn't have (one @@ -1230,7 +1525,21 @@ impl<'g> Enc<'g> { std::mem::take(&mut self.missing) } - pub(crate) fn encoder(&mut self) -> &mut wgpu::CommandEncoder { + /// The command encoder, to record work that touches `imgs` (every pooled image the commands + /// read or write; buffers and textures of the caller's own need not be named). The only + /// way to record outside this module: naming the images marks foreign leases escaped + /// ([`Enc::uses`]). Callers must name every pooled image the encoder's work uses, here or + /// with separate [`Enc::uses`] calls (`record(&[])` plus `uses()` is allowed); this is a + /// convention, not enforced, and skipping it reintroduces the reuse race it guards (#1441). + pub(crate) fn record(&mut self, imgs: &[&GpuImage]) -> &mut wgpu::CommandEncoder { + for img in imgs { + self.uses(img); + } + self.encoder() + } + + /// The raw encoder: within this module only, after [`Enc::uses`] on every pooled image. + fn encoder(&mut self) -> &mut wgpu::CommandEncoder { let g = self.g; &mut self .enc @@ -1329,7 +1638,8 @@ impl<'g> Enc<'g> { ], }) }); - let enc = self.encoder(); + let imgs: Vec<&GpuImage> = [Some(src), aux, Some(out)].into_iter().flatten().collect(); + let enc = self.record(&imgs); { let mut pass = enc.begin_compute_pass(&wgpu::ComputePassDescriptor { label: Some(entry), timestamp_writes: None }); pass.set_pipeline(pipe); @@ -1354,7 +1664,7 @@ impl<'g> Enc<'g> { /// Copy `src` into a larger zeroed image at (`x`, `y`) (Buf::pad). pub fn copy_into(&mut self, src: &GpuImage, dst: &GpuImage, x: u32, y: u32) { - let enc = self.encoder(); + let enc = self.record(&[src, dst]); enc.copy_texture_to_texture( wgpu::TexelCopyTextureInfo { texture: &src.texture, mip_level: 0, origin: wgpu::Origin3d::ZERO, aspect: wgpu::TextureAspect::All }, wgpu::TexelCopyTextureInfo { texture: &dst.texture, mip_level: 0, origin: wgpu::Origin3d { x, y, z: 0 }, aspect: wgpu::TextureAspect::All }, @@ -1371,7 +1681,7 @@ impl<'g> Enc<'g> { (b > a).then(|| (a as u32, (a - o) as u32, (b - a) as u32)) }; let (Some((sx, dx, cw)), Some((sy, dy, ch))) = (span(x, w, src.width), span(y, h, src.height)) else { return dst }; - let enc = self.encoder(); + let enc = self.record(&[src, &dst]); enc.copy_texture_to_texture( wgpu::TexelCopyTextureInfo { texture: &src.texture, mip_level: 0, origin: wgpu::Origin3d { x: sx, y: sy, z: 0 }, aspect: wgpu::TextureAspect::All }, wgpu::TexelCopyTextureInfo { texture: &dst.texture, mip_level: 0, origin: wgpu::Origin3d { x: dx, y: dy, z: 0 }, aspect: wgpu::TextureAspect::All }, @@ -1393,6 +1703,7 @@ impl<'g> Enc<'g> { /// Read an image back to the CPU (submits and waits). `None` where readback is impossible. pub fn download(&mut self, img: &GpuImage) -> Option { + self.uses(img); let layout = crate::readback::Layout::new(img.width, img.height, 16, self.g.device.limits().max_buffer_size) .map_err(|e| log::error!("gpu image readback: {e}")) .ok()?; @@ -1509,6 +1820,7 @@ impl<'g> Enc<'g> { /// once they arrived (`Some(None)`: the readback failed), else `None` after starting the /// copy if needed and marking the pass missed. pub(crate) fn read_keyed(&mut self, img: &GpuImage, key: u128, offset: [f64; 2], scale: f64) -> Option> { + self.uses(img); use crate::deferred::Lookup; let d = self.g.deferred.clone()?; match d.lookup(key) { @@ -1536,6 +1848,7 @@ impl<'g> Enc<'g> { /// Copy an image into a storage buffer (RGBA f32 rows) for kernels that read a third image /// through `data`: (buffer, row length in pixels). pub fn image_rows(&mut self, img: &GpuImage) -> (wgpu::Buffer, u32) { + self.uses(img); let row = (img.width * 16).div_ceil(wgpu::COPY_BYTES_PER_ROW_ALIGNMENT) * wgpu::COPY_BYTES_PER_ROW_ALIGNMENT; let buf = self.g.device.create_buffer(&wgpu::BufferDescriptor { label: Some("image rows"), @@ -1565,6 +1878,7 @@ impl<'g> Enc<'g> { /// Convert to the viewer's premultiplied RGBA8 texture (egui-wgpu's native texture format), /// with a full mip chain so the viewer minifies it without aliasing (#417). pub fn display(&mut self, img: &GpuImage) -> wgpu::Texture { + self.uses(img); let g = self.g; let levels = mip_levels(img.width, img.height); let tex = g.device.create_texture(&wgpu::TextureDescriptor { @@ -1607,6 +1921,30 @@ pub(crate) fn mip_levels(w: u32, h: u32) -> u32 { u32::BITS - w.max(h).max(1).leading_zeros() } +#[cfg(test)] +mod pool_stall_tests { + use super::Pool; + use std::sync::atomic::Ordering::Relaxed; + + #[test] + fn frame_end_skips_wait_while_stalled_until_dying_drops() { + let mut p = Pool::default(); + p.dying.store(100, Relaxed); + assert_eq!(p.frame_end(), 100); + p.waited(100); + assert_eq!(p.stalled, 100); + assert_eq!(p.waits, 1); + assert_eq!(p.frame_end(), 0); + p.dying.store(150, Relaxed); + assert_eq!(p.frame_end(), 0); + p.dying.store(50, Relaxed); + assert_eq!(p.frame_end(), 50); + assert_eq!(p.stalled, 0); + p.dying.store(0, Relaxed); + assert_eq!(p.frame_end(), 0); + } +} + #[cfg(test)] mod adapter_name_tests { use super::{match_adapter_name, wanted_adapter_name}; diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index c58ccd59e..d0a688e49 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -397,7 +397,7 @@ impl Accelerator for Gpu { fn effects(&self, chain: &[FxStep], buf: &Buf, levels: Option) -> Option { self.ctx.check_health().ok()?; - effects::run_chain(&mut Enc::new(&self.ctx), chain, buf, levels) + effects::run_chain(&mut Enc::lent(&self.ctx), chain, buf, levels) } fn raster_3d(&self, scene: &effectcraft_render::three_d::adv::Scene) -> Option { @@ -419,12 +419,26 @@ impl Accelerator for Gpu { Some(&self.auto) } + fn render_end(&self) { + let (hits, misses) = self.ctx.pool_stats(); + let pct = hits.saturating_mul(100) / hits.saturating_add(misses).max(1); + log::debug!( + target: "effectcraft_gpu", + "gpu pool: {hits} working textures reused, {misses} allocated ({pct}% reuse) so far" + ); + self.ctx.trim_pool(); + } + fn frame_begin(&self) { if let Some(d) = self.ctx.deferred() { d.frame_begin(); } } + fn frame_end(&self) { + self.ctx.pool_frame_end(); + } + fn pass_begin(&self) { if let Some(d) = self.ctx.deferred() { d.pass_begin(); diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 44e8922a6..28c27b5c7 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -784,7 +784,7 @@ fn display_frames_are_mipmapped() { view_formats: &[], }); let mut e = crate::context::Enc::new(&g.ctx); - e.encoder().copy_texture_to_texture( + e.record(&[]).copy_texture_to_texture( wgpu::TexelCopyTextureInfo { texture: &f.texture, mip_level: 1, origin: wgpu::Origin3d::ZERO, aspect: wgpu::TextureAspect::All }, wgpu::TexelCopyTextureInfo { texture: &tex, mip_level: 0, origin: wgpu::Origin3d::ZERO, aspect: wgpu::TextureAspect::All }, wgpu::Extent3d { width: w1, height: h1, depth_or_array_layers: 1 }, @@ -920,7 +920,7 @@ fn a_failed_frame_returns_no_texture_to_the_pool() { let rejected = rejected_texture(&g); let earlier = g.within_memory(|| crate::context::Enc::new(&g.ctx).image(48, 16)).expect("an earlier frame"); let mut other = crate::context::Enc::new(&g.ctx); - other.encoder(); + other.record(&[]); g.within_memory(|| (crate::context::Enc::new(&g.ctx).image(64, 32), rejected.create_view(&Default::default()))).expect_err("the frame fails"); drop(earlier); drop(other); @@ -1315,3 +1315,274 @@ fn owned_devices_favour_memory_usage() { assert_eq!(d.label, Some("effectcraft gpu")); assert_eq!(d.required_limits.max_texture_dimension_2d, wgpu::Limits::default().max_texture_dimension_2d); } + +/// A GPU context of its own with a small pool budget, so other tests (sharing [`gpu`]) neither +/// see the budget nor add to what the pool keeps. +fn private_gpu(budget: usize) -> Option { + gpu()?; + let g = Gpu::headless()?; + g.context().set_pool_budget(budget); + Some(g) +} + +/// Three effect calls while a frame encoder is open with recorded work reuse one set of +/// textures: lent calls give them back at once, without waiting for that frame to submit. +#[test] +fn three_effect_calls_in_a_frame_reuse_one_set_of_textures() { + let Some(g) = private_gpu(8 << 20) else { return }; + let ctx = g.context(); + let mut frame = crate::context::Enc::new(ctx); + // Open the frame encoder with real work so the pool has an open id (an empty Enc::new does not). + let src = frame.image(64, 64); + let dst = frame.image(64, 64); + frame.copy_into(&src, &dst, 0, 0); + let (hits0, misses0) = ctx.pool_stats(); + for _ in 0..3 { + let mut e = crate::context::Enc::lent(ctx); + let a = e.image(256, 256); + let b = e.image(256, 256); + drop((a, b)); + drop(e); + } + let (hits, misses) = (ctx.pool_stats().0 - hits0, ctx.pool_stats().1 - misses0); + assert_eq!(misses, 2, "only the first effect call allocates: {hits} hits, {misses} misses"); + assert_eq!(hits, 4, "calls 2 and 3 reuse both textures: {hits} hits, {misses} misses"); + frame.submit(); + drop((src, dst, frame)); +} + +/// Review of #1523: after the first frame fills the free list, later identical frames hit +/// instead of allocating again. +#[test] +fn a_repeated_frame_reuses_after_the_first() { + let Some(g) = private_gpu(8 << 20) else { return }; + let ctx = g.context(); + let frame = || { + let mut e = crate::context::Enc::lent(ctx); + drop((e.image(256, 256), e.image(128, 64))); + drop(e); + ctx.pool_frame_end(); + }; + let (hits0, misses0) = ctx.pool_stats(); + frame(); + let (hits1, misses1) = (ctx.pool_stats().0 - hits0, ctx.pool_stats().1 - misses0); + assert_eq!(misses1, 2, "first frame allocates both sizes: {hits1} hits, {misses1} misses"); + let (hits_a, misses_a) = ctx.pool_stats(); + for _ in 0..3 { + frame(); + } + let (hits, misses) = (ctx.pool_stats().0 - hits_a, ctx.pool_stats().1 - misses_a); + assert_eq!(misses, 0, "frames 2..4 allocate nothing: {hits} hits, {misses} misses"); + assert!(hits >= 6, "frames 2..4 reuse both sizes: {hits} hits"); +} + +/// Review of #1523: every idle effect-call owner kept up to the pool budget of its own, +/// outside the shared accounting, and there are as many as calls ever ran at once (16 frame +/// threads: ~12 GB after a 4K render). Finished calls share the free list's one budget, and +/// the end of a render drops it. +#[test] +fn idle_effect_calls_share_one_pool_budget() { + let budget = 8 << 20; + let Some(g) = private_gpu(budget) else { return }; + let ctx = g.context(); + // 16 effect calls in flight at once, each with three 1 MB working textures. + let mut encs: Vec<_> = (0..16).map(|_| crate::context::Enc::lent(ctx)).collect(); + for e in &mut encs { + let imgs: Vec<_> = (0..3).map(|_| e.image(256, 256)).collect(); + drop(imgs); + } + drop(encs); + let kept = ctx.pool_retained_bytes(); + assert!(kept <= budget, "idle effect calls keep {} MB, budget {} MB", kept >> 20, budget >> 20); + // A later call reuses them instead of allocating. + let (hits, _) = ctx.pool_stats(); + drop(crate::context::Enc::lent(ctx).image(256, 256)); + assert_eq!(ctx.pool_stats().0, hits + 1, "a later effect call reuses a kept texture"); + effectcraft_render::Accelerator::render_end(&g); + assert_eq!(ctx.pool_retained_bytes(), 0, "the end of a render drops the kept textures"); +} + +/// Review of #1523: a full free list rejected every new texture instead of evicting, so once +/// sizes that no longer occur (per-frame blur bounds, padded layer sizes) filled it, a steady +/// size never got a hit again: ~0.75 GB of dead weight plus an allocation per working texture. +#[test] +fn stale_sizes_do_not_starve_a_steady_size() { + let budget = 8 << 20; + let Some(g) = private_gpu(budget) else { return }; + let ctx = g.context(); + for k in 0..16 { + let mut e = crate::context::Enc::lent(ctx); + drop((e.image(256 + k, 256), e.image(256 + k, 256))); + drop(e); + ctx.pool_frame_end(); + } + let (hits0, misses0) = ctx.pool_stats(); + for _ in 0..10 { + let mut e = crate::context::Enc::lent(ctx); + drop((e.image(300, 300), e.image(300, 300))); + drop(e); + ctx.pool_frame_end(); + } + let (hits, misses) = (ctx.pool_stats().0 - hits0, ctx.pool_stats().1 - misses0); + assert!(ctx.pool_retained_bytes() <= budget); + assert!(ctx.pool_evictions() > 0, "the stale sizes were evicted"); + assert!(hits >= 18 && misses <= 2, "steady 300x300 calls after stale sizes: {hits} hits, {misses} misses"); +} + +/// Review of #1523: a `pool_frame_end` wait that clears none of the evicted bytes (timeout, or +/// callbacks that never run) must not re-wait on every later frame while `dying` stays put. +#[test] +fn a_wait_without_progress_is_not_repeated() { + let Some(g) = private_gpu(8 << 20) else { return }; + let ctx = g.context(); + let phantom = 1 << 20; + ctx.pool_charge_dying(phantom as isize); + ctx.pool_frame_end(); + assert_eq!(ctx.pool_waits(), 1, "first frame with dying bytes waits once"); + for _ in 0..3 { + ctx.pool_frame_end(); + } + assert_eq!(ctx.pool_waits(), 1, "no progress: later frames must not wait again"); + ctx.pool_charge_dying(-(phantom as isize)); + ctx.pool_frame_end(); + assert_eq!(ctx.pool_waits(), 1, "dying cleared: poll only, stall cleared"); + ctx.pool_charge_dying(phantom as isize); + ctx.pool_frame_end(); + assert_eq!(ctx.pool_waits(), 2, "a new stall waits once"); + // Partial progress: more dying while stalled → still no wait; drop below stalled → wait. + let extra = phantom / 2; + ctx.pool_charge_dying(extra as isize); + ctx.pool_frame_end(); + assert_eq!(ctx.pool_waits(), 2, "dying above stalled: poll only"); + ctx.pool_charge_dying(-(extra as isize + 1)); + ctx.pool_frame_end(); + assert_eq!(ctx.pool_waits(), 3, "dying dropped below stalled: wait again"); + ctx.pool_charge_dying(-((phantom - 1) as isize)); +} + +/// RTX 3090 4K runs of #1523: evicting the least recently freed texture to fit every newcomer +/// thrashes when a frame frees more than the budget in a repeating order (blur/glow +/// intermediates): each free evicts the size the next call takes first, so every take +/// missed (d_blurglow misses back at the old level, and evict + reallocate raised the peak). +/// Sizes still in use stay; a kept subset of the cycle must keep hitting. +#[test] +fn repeating_sizes_over_the_budget_keep_hitting() { + let budget = 8 << 20; + let Some(g) = private_gpu(budget) else { return }; + let ctx = g.context(); + // Five ~2.1 MB sizes a frame (10.5 MB > 8 MB), one effect call each, same order every frame. + let sizes: Vec<(u32, u32)> = (0..5).map(|k| (256 + 8 * k, 512)).collect(); + let frame = || { + for &(w, h) in &sizes { + drop(crate::context::Enc::lent(ctx).image(w, h)); + } + ctx.pool_frame_end(); + }; + for _ in 0..3 { + frame(); + } + let (hits0, misses0) = ctx.pool_stats(); + let evicted0 = ctx.pool_evictions(); + for _ in 0..10 { + frame(); + } + let (hits, misses) = (ctx.pool_stats().0 - hits0, ctx.pool_stats().1 - misses0); + assert!(ctx.pool_retained_bytes() <= budget); + assert!(hits >= 30, "10 frames of a 5-size cycle over the budget: {hits} hits, {misses} misses"); + assert_eq!(ctx.pool_evictions(), evicted0, "sizes in use were evicted"); +} + +/// Review of #1523: a lent call's image read by another, still open encoder must not go back to +/// the lent call for reuse when its lease drops first: that encoder's read is not submitted yet. +#[test] +fn image_read_by_an_open_encoder_is_not_reused_by_its_lent_call() { + let Some(g) = private_gpu(8 << 20) else { return }; + let ctx = g.context(); + let mut frame = crate::context::Enc::new(ctx); + let mut lent = crate::context::Enc::lent(ctx); + let img = lent.image(64, 64); + let dst = frame.image(64, 64); + frame.copy_into(&img, &dst, 0, 0); + drop(img); + let (hits, _) = ctx.pool_stats(); + drop(lent.image(64, 64)); + assert_eq!(ctx.pool_stats().0, hits, "the lent call reused a texture an open encoder still reads"); + drop(lent); + let (hits, _) = ctx.pool_stats(); + drop(crate::context::Enc::lent(ctx).image(64, 64)); + // The other 64x64 (the lent call's second image) is free; the read one still waits. + let (hits2, _) = ctx.pool_stats(); + assert_eq!(hits2, hits + 1); + let (h, _) = ctx.pool_stats(); + let mut e = crate::context::Enc::lent(ctx); + let a = e.image(64, 64); + let b = e.image(64, 64); + assert_eq!(ctx.pool_stats().0, h + 1, "the read texture waits for the open encoder"); + drop((a, b, e)); + frame.submit(); + drop(dst); +} + +/// The same through [`Gpu::effects`] from 16 threads at once, as an export's frame threads. +#[test] +fn effect_calls_on_16_threads_keep_one_pool_budget() { + if on_gl() { + return; // one context lock per adapter on wgpu's GL backend (see hold_gpu_lock) + } + let budget = 8 << 20; + let Some(g) = private_gpu(budget) else { return }; + let g = &g; + let ctx = g.context(); + let spec = effectcraft_effects::find("ec.blur.gaussian").unwrap(); + let size = [240.0, 240.0]; + let mut params = + effectcraft_effects::Params { values: spec.params.iter().map(|ps| (ps.id.to_string(), effectcraft_effects::default_value(ps, size))).collect() }; + params.values.insert("blurriness".into(), Value::Scalar(6.0)); + let buf = effectcraft_effects::Buf { img: pattern(3, 240, 240), offset: [0.0, 0.0], scale: 1.0 }; + let go = std::sync::Barrier::new(16); + std::thread::scope(|s| { + for i in 0..16 { + let (params, buf, go) = (¶ms, &buf, &go); + s.spawn(move || { + go.wait(); + for _ in 0..3 { + let fx = effectcraft_effects::EffectCtx { params, time: 0.0, layer_size: size, seed: i, adjustment: false, env: Default::default() }; + effectcraft_render::Accelerator::effects(g, &[effectcraft_render::FxStep { spec, ctx: fx }], buf, None).expect("the GPU runs the effect"); + } + }); + } + }); + let kept = ctx.pool_retained_bytes(); + assert!(kept <= budget, "16 threads of effect calls keep {} MB, budget {} MB", kept >> 20, budget >> 20); + effectcraft_render::Accelerator::render_end(g); + assert_eq!(ctx.pool_retained_bytes(), 0, "the end of a render drops the kept textures"); +} + +/// Sceptic r2 of #1523: only the export pipeline ended frames, so off export (preview, remote +/// frames) the pool's clock stood still, no size ever went stale and a full free list dropped +/// every newcomer again. Frames rendered through [`effectcraft_render::passes::comp_frame`] +/// (no explicit `pool_frame_end`) must age the sizes they did not ask for. +#[test] +fn frames_outside_the_export_pipeline_age_the_pool() { + let budget = 8 << 20; + let Some(g) = private_gpu(budget) else { return }; + let ctx = g.context(); + // Fill the free list with sizes no frame asks for again. + for k in 0..16 { + let mut e = crate::context::Enc::lent(ctx); + drop((e.image(256 + k, 256), e.image(256 + k, 256))); + } + let evicted0 = ctx.pool_evictions(); + let s = blend_scene(BitDepth::Bpc8, BlendMode::Screen); + let mut r = Renderer::new(&s.p, &Pattern, opts()); + r.accel = Some(&g); + for _ in 0..3 { + effectcraft_render::passes::block_on(effectcraft_render::passes::comp_frame(&r, s.cid, Tick::ZERO)); + } + for _ in 0..4 { + let mut e = crate::context::Enc::lent(ctx); + drop((e.image(300, 300), e.image(300, 300))); + } + assert!(ctx.pool_retained_bytes() <= budget); + assert!(ctx.pool_evictions() > evicted0, "rendered frames did not age the pool: stale sizes were never evicted"); +} diff --git a/crates/gpu/src/yuv.rs b/crates/gpu/src/yuv.rs index aba3ec700..27df90110 100644 --- a/crates/gpu/src/yuv.rs +++ b/crates/gpu/src/yuv.rs @@ -130,7 +130,7 @@ pub(crate) fn record(e: &mut Enc, yp: &YuvPipe, img: &GpuImage, bg: [f32; 3]) -> ], }); { - let enc = e.encoder(); + let enc = e.record(&[img]); { let mut pass = enc.begin_compute_pass(&wgpu::ComputePassDescriptor { label: Some("yuv420"), timestamp_writes: None }); pass.set_pipeline(&yp.pipe); diff --git a/crates/render/src/accel.rs b/crates/render/src/accel.rs index e6bbd2113..9b7d29ec6 100644 --- a/crates/render/src/accel.rs +++ b/crates/render/src/accel.rs @@ -96,6 +96,18 @@ pub trait Accelerator: Send + Sync { /// results, `pass_begin` clears the miss. Synchronous accelerators never miss. fn frame_begin(&self) {} fn pass_begin(&self) {} + /// A rendered comp frame is done (the GPU pool ages sizes no frame asks for, and evicts + /// only those once full). The multi-frame render paths call it once per comp frame: + /// [`crate::passes::comp_frame`] (exports; a field-rendered output frame is two), remote frames + /// and the preview workers. One-off renders (`Session::render`, single-frame export, + /// thumbnails) don't, so they don't advance the pool's clock. Without it the pool never + /// finds a stale size and keeps its first sizes, dropping newcomers. + fn frame_end(&self) {} + /// A render of many frames (an export) is done: drop what is kept only to speed up the + /// next frame (pooled working textures). Exports call it, even when they fail, panic or + /// are dropped; other multi-frame paths (preview, remote frames) don't, so there the pool + /// keeps up to its budget until the next export ends. + fn render_end(&self) {} fn pass_missed(&self) -> bool { false } diff --git a/crates/render/src/passes.rs b/crates/render/src/passes.rs index 5f0373b84..37c94fff4 100644 --- a/crates/render/src/passes.rs +++ b/crates/render/src/passes.rs @@ -59,11 +59,25 @@ pub async fn in_passes<'a, T>(accel: Option<&'a dyn Accelerator>, mut render: im Passed { value: render(None), passes: MAX_PASSES + 1, accelerated: false } } +/// Ends the frame for the accelerator ([`Accelerator::frame_end`]) when dropped: also when the +/// render panics or the future is dropped between passes. +struct FrameEnd<'a>(&'a dyn Accelerator); +impl Drop for FrameEnd<'_> { + fn drop(&mut self) { + self.0.frame_end(); + } +} + /// A top-level comp frame through [`in_passes`] ([`Renderer::comp_frame`] in passes). Backend /// Auto picks CPU or GPU for the whole frame (all its passes) and is timed as a whole, as the -/// frame workers do. +/// frame workers do. Ends the frame for the accelerator ([`Accelerator::frame_end`]) on every +/// exit, including a panic. pub async fn comp_frame(r: &Renderer<'_>, comp: ItemId, t: Tick) -> Image { - let Some(a) = r.accel.filter(|a| a.miss_gate().is_some()) else { return r.comp_frame(comp, t) }; + let Some(a) = r.accel else { return r.comp_frame(comp, t) }; + let _end = FrameEnd(a); + if a.miss_gate().is_none() { + return r.comp_frame(comp, t); + } let mut opts = r.opts; let mut auto = None; if opts.backend == Backend::Auto { diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 865a4f59f..c77fc2d94 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -838,6 +838,50 @@ impl crate::Accelerator for ProbeAccel { } } +/// Counts [`Accelerator::frame_end`] and optionally panics inside [`Accelerator::comp_frame`]. +struct EndAccel { + ends: std::sync::atomic::AtomicUsize, + panic: std::sync::atomic::AtomicBool, +} +impl crate::Accelerator for EndAccel { + fn name(&self) -> String { + "end".into() + } + fn comp_frame(&self, _r: &crate::Renderer, _comp: ItemId, _t: Tick) -> Option { + if self.panic.load(std::sync::atomic::Ordering::SeqCst) { + panic!("comp_frame"); + } + Some(crate::Image::new(1, 1)) + } + fn supports_effect(&self, _id: &str) -> bool { + false + } + fn effects(&self, _chain: &[crate::FxStep], _buf: &crate::Buf, _levels: Option) -> Option { + None + } + fn frame_end(&self) { + self.ends.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + } +} + +#[test] +fn comp_frame_ends_the_frame_when_the_render_panics() { + use std::panic::{AssertUnwindSafe, catch_unwind}; + use std::sync::atomic::Ordering; + let (mut p, cid, comp) = setup(); + let l = solid(&mut p, &comp, [1.0, 0.0, 0.0], 50, 50); + p.comp_mut(cid).unwrap().layers.push(l); + let a = EndAccel { ends: Default::default(), panic: true.into() }; + let mut r = crate::Renderer::new(&p, &crate::NoFootage, crate::RenderOpts { backend: crate::Backend::Gpu, ..Default::default() }); + r.accel = Some(&a); + assert!(catch_unwind(AssertUnwindSafe(|| crate::passes::block_on(crate::passes::comp_frame(&r, cid, Tick::ZERO)))).is_err()); + assert_eq!(a.ends.load(Ordering::SeqCst), 1); + a.panic.store(false, Ordering::SeqCst); + a.ends.store(0, Ordering::SeqCst); + let _ = crate::passes::block_on(crate::passes::comp_frame(&r, cid, Tick::ZERO)); + assert_eq!(a.ends.load(Ordering::SeqCst), 1); +} + #[test] fn auto_backend_sends_2d_and_3d_comps_to_the_gpu() { let (mut p, cid, comp) = setup(); diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 87cd9a17b..9aaaedbc7 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -1382,10 +1382,15 @@ impl Worker { /// `None` if that panicked too. fn render_guarded(&self, job: &Job) -> Option { let attempt = |gpu: bool| std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| self.render_job(job, gpu))).ok(); - attempt(true).or_else(|| { + let img = attempt(true).or_else(|| { log::error!("a frame render panicked (frame {})", job.key.frame); job.src.gpu.as_ref().and_then(|_| attempt(false)) - }) + }); + // A preview frame is done: the GPU pool ages the sizes it did not ask for. + if let Some(g) = &job.src.gpu { + effectcraft_engine::render::Accelerator::frame_end(g); + } + img } /// A failed scoped attempt retries on CPU. Retired contexts cannot recover by reducing @@ -1418,7 +1423,9 @@ impl Worker { r.cache = Some(&job.src.layer_cache); r.accel = Some(g as &dyn effectcraft_engine::render::Accelerator); r.active_accel()?; - match g.within_memory(|| g.render_display(&r, job.comp, job.t)) { + let f = g.within_memory(|| g.render_display(&r, job.comp, job.t)); + effectcraft_engine::render::Accelerator::frame_end(g); + match f { Ok(f) => f.map(|f| FrameImage::Gpu(Arc::new(f))), Err(e) => { self.gpu_attempt_failed(g, &e);