diff --git a/crates/engine/src/background/remote.rs b/crates/engine/src/background/remote.rs index aea210ec5..72b4598a2 100644 --- a/crates/engine/src/background/remote.rs +++ b/crates/engine/src/background/remote.rs @@ -430,7 +430,10 @@ impl FrameServer { } } // A frame still waiting for readbacks is replaced: end it, so every `frame_begin` has its `frame_end`. - if self.pending.take().is_some() { + if let Some(old) = self.pending.take() { + if let (Some((key, chose)), Some(pick)) = (old.auto, a.auto_pick()) { + pick.abandoned(key, chose); + } a.frame_end(); } a.frame_begin(); diff --git a/crates/render/src/auto.rs b/crates/render/src/auto.rs index b1138588b..fc7becb5d 100644 --- a/crates/render/src/auto.rs +++ b/crates/render/src/auto.rs @@ -67,9 +67,7 @@ pub struct AutoPick { struct Entry { stats: AutoStats, measuring: [Option; 2], - // ponytail: a frame that never reports back (a GPU failure path, a replaced remote frame) - // leaves its side counted as running, which only makes that side look cheaper; capped at - // MAX_RUNNING, cleared by `AutoPick::reset`. + // Lowered by `record`, `declined` and `abandoned`: every frame asked for ends in one. running: [u32; 2], } @@ -109,7 +107,7 @@ impl AutoPick { const MEASURING_FOR: std::time::Duration = std::time::Duration::from_secs(10); /// Render the frame for `key` on the GPU? Every frame asked for reports back with - /// [`AutoPick::record`] (or [`AutoPick::declined`]). + /// [`AutoPick::record`] (or [`AutoPick::declined`]), or ends with [`AutoPick::abandoned`]. pub fn choose(&self, key: AutoKey) -> bool { let Ok(mut m) = self.stats.lock() else { return true }; if m.len() > Self::MAX_KEYS && !m.contains_key(&key) { @@ -194,6 +192,21 @@ impl AutoPick { } } + /// A frame for `key` asked of `gpu`'s side ends without a time (it failed, or was replaced + /// before it finished). Without this its side stays counted as running, and every later + /// frame there is timed as one of several running side by side: that side looks cheaper. + // ponytail: clears the side's warm-up/probe mark even when another frame holds it; that + // only lets the next probe start early. + pub fn abandoned(&self, key: AutoKey, gpu: bool) { + if let Ok(mut m) = self.stats.lock() + && let Some(e) = m.get_mut(&key) + { + e.measuring[usize::from(gpu)] = None; + let n = &mut e.running[usize::from(gpu)]; + *n = n.saturating_sub(1); + } + } + /// The history of `key`. pub fn stats(&self, key: AutoKey) -> Option { self.stats.lock().ok()?.get(&key).map(|e| e.stats) diff --git a/crates/render/src/pipeline.rs b/crates/render/src/pipeline.rs index fd46260c2..a77f8661d 100644 --- a/crates/render/src/pipeline.rs +++ b/crates/render/src/pipeline.rs @@ -587,7 +587,13 @@ impl<'a> Renderer<'a> { let auto = self.frame_auto(a, comp_id, false); if auto.is_none_or(|(p, key)| p.choose(key)) { let t0 = web_time::Instant::now(); - if let Some(img) = scope(a)? { + let res = scope(a); + if res.is_err() + && let Some((p, key)) = auto + { + p.abandoned(key, true); + } + if let Some(img) = res? { if let Some((p, key)) = auto { p.record(key, true, t0.elapsed().as_secs_f64() * 1e3); } diff --git a/crates/render/src/tests/auto.rs b/crates/render/src/tests/auto.rs index e7387eed3..da3d9f348 100644 --- a/crates/render/src/tests/auto.rs +++ b/crates/render/src/tests/auto.rs @@ -308,3 +308,23 @@ fn auto_pick_counts_frames_running_side_by_side() { assert!(s.cpu_ms.unwrap() < 30.0 && s.gpu_ms.unwrap() > 100.0, "{s:?}"); assert!(lanes.iter().filter(|g| **g).count() <= 1, "{lanes:?}"); } + +/// A frame that ends without a time (a GPU failure, a remote frame replaced before it finished) +/// no longer counts as running: the frames after it are timed as one at a time. (It stayed +/// counted, and every later frame on its side was timed as half its cost.) +#[test] +fn auto_pick_abandoned_frames_stop_counting_as_running() { + let pick = AutoPick::default(); + let key = AutoKey::new(ItemId(5), 1.0, false); + let mut dropped = [false; 2]; + for _ in 0..16 { + let gpu = pick.choose(key); + if !std::mem::replace(&mut dropped[usize::from(gpu)], true) { + pick.abandoned(key, gpu); + continue; + } + pick.record(key, gpu, if gpu { 40.0 } else { 10.0 }); + } + let s = pick.stats(key).unwrap(); + assert_eq!((s.cpu_ms, s.gpu_ms), (Some(10.0), Some(40.0)), "{s:?}"); +} diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 2d51068b0..01b9824d6 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -2019,6 +2019,7 @@ impl Worker { // Time the GPU's work, not just its recording. g.wait(); if g.context().check_health().is_err() { + p.abandoned(key, true); return self.render_job(job, false, files); } p.record(key, true, t0.elapsed().as_secs_f64() * 1000.0); @@ -2031,6 +2032,9 @@ impl Worker { } } Err(e) => { + if let Some((p, key)) = auto { + p.abandoned(key, true); + } self.gpu_attempt_rejected(g, &e); return self.render_job(job, false, files); }