From 88f424e7e70c22cbaf2914e2fb20ffcbcadc860f Mon Sep 17 00:00:00 2001 From: akvaithi Date: Sat, 10 Oct 2026 15:53:39 -0500 Subject: [PATCH] LR-EDIT-DETAIL-DENOISE: share tiles with a server only when it is faster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured with RawNIND on a 20.8 MP raw: an Apple M3 does a tile in 116 ms, an RTX 3060 Ti over Tailscale 345 ms there and back; sharing blindly made the export take 34 s instead of 6.4 s while the photo waited for the server's last tiles. The greeting now times a check tile there and back, and tiles are shared only when that beats this computer's own time per tile (the card's, or the CPU's with its threads counted as `card_is_faster` does). Otherwise `denoise.status` → `device.remote` says `slower` with both times. With the server configured but slower the export takes 7.2 s (greeting + check tile). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/denoise.rs | 2 +- crates/engine/src/denoise.rs | 97 +++++++++++++++++++++++++++----- docs/denoise.md | 11 +++- 3 files changed, 91 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/cmd/denoise.rs b/crates/engine/src/cmd/denoise.rs index 81c9eb39d..8802460a1 100644 --- a/crates/engine/src/cmd/denoise.rs +++ b/crates/engine/src/cmd/denoise.rs @@ -653,7 +653,7 @@ pub fn specs() -> Vec { ), cmd!(query "denoise.queue", "Denoise Photos", [], None, "{ids?: [photoId], scope?: selected | visible | withAmount, retry?: false} → {added, …status} — make the denoised pictures of raw photos that have none, in the background. `retry` tries photos that failed before", always, queue), cmd!(query "denoise.cancel", "Cancel Denoise", [], None, "{id?, all?} → {cancelled} — stop the photo being made and/or forget the ones waiting (all of them when `id` is omitted)", always, cancel), - cmd!(query "denoise.status", "Denoise Status", [], None, "{id?} → {enabled, model, runtime, productsDir, queued, running: {photo, done, total, seconds} | null, ready, made, auto, runOn, device: {kind: gpu | cpu | pending | none, adapter?, reason?, cardMs?, cpuMs?, fellBack?, remote?: {address, state: pending | unavailable | used | given up, device?, reason?, failures?, lastError?}}, cache: {files, bytes, limitBytes}, generation, photo?: {state: notApplicable | noModel | ready | queued | running | failed | idle, …}}", always, status), + cmd!(query "denoise.status", "Denoise Status", [], None, "{id?} → {enabled, model, runtime, productsDir, queued, running: {photo, done, total, seconds} | null, ready, made, auto, runOn, device: {kind: gpu | cpu | pending | none, adapter?, reason?, cardMs?, cpuMs?, fellBack?, remote?: {address, state: pending | unavailable | used | idle | slower | given up, device?, remoteMs?, localMs?, reason?, failures?, lastError?}}, cache: {files, bytes, limitBytes}, generation, photo?: {state: notApplicable | noModel | ready | queued | running | failed | idle, …}}", always, status), cmd!(query "denoise.pump", "Denoise Pump", [], None, "{pace?: pause | light | normal | full} → {active, running, queued, ready, generation, downloading} — called every frame by the app: takes in finished work and keeps one photo going", always, pump), cmd!(query "denoise.clear", "Clear Denoise Cache", [], None, "{} → {deleted, bytes} — delete every cached denoised picture of this library (they are made again when wanted)", always, clear), ] diff --git a/crates/engine/src/denoise.rs b/crates/engine/src/denoise.rs index 8e939e611..33582aaed 100644 --- a/crates/engine/src/denoise.rs +++ b/crates/engine/src/denoise.rs @@ -168,7 +168,11 @@ struct Network { #[cfg(feature = "denoise")] struct RemoteSide { runner: lightcraft_denoise::remote::Remote, - greeted: OnceLock>, + /// The greeting, and the server's time for the check tile there and back (ms; the faster of two runs). + greeted: OnceLock>, + /// Whether the photo started last shares tiles with the server, and if not because it is slower, the two times. + share: AtomicBool, + slower: Mutex>, } #[cfg(feature = "denoise")] @@ -178,12 +182,33 @@ impl RemoteSide { let var = |k: &str| std::env::var(k).ok().map(|v| v.trim().to_string()).filter(|v| !v.is_empty()); let address = var("LIGHTCRAFT_DENOISE_URL")?; let token = var("LIGHTCRAFT_DENOISE_TOKEN").unwrap_or_default(); - Some(RemoteSide { runner: lightcraft_denoise::remote::Remote::new(&address, &token, sha256, tile), greeted: OnceLock::new() }) + Some(RemoteSide::new(lightcraft_denoise::remote::Remote::new(&address, &token, sha256, tile))) + } + + fn new(runner: lightcraft_denoise::remote::Remote) -> RemoteSide { + RemoteSide { runner, greeted: OnceLock::new(), share: AtomicBool::new(false), slower: Mutex::new(None) } + } + + /// Greet the server and time a check tile there and back: a server behind a slow link can take seconds a tile, and + /// handing it tiles a fast local card would finish sooner only makes the photo wait for it. + fn greet(&self) -> Result<(lightcraft_denoise::remote::ServerInfo, f64), String> { + let info = self.runner.hello().map_err(|e| e.to_string())?; + let input = lightcraft_denoise::runtime::check_tile(info.tile); + let mut best = f64::INFINITY; + for _ in 0..2 { + let started = web_time::Instant::now(); + match self.runner.try_run(&input) { + Some(Ok(_)) => best = best.min(started.elapsed().as_secs_f64() * 1000.0), + Some(Err(e)) => return Err(format!("the check tile failed: {e}")), + None => return Err("no free slot for the check tile".into()), + } + } + Ok((info, best)) } /// The server when it can take tiles now (greeting it the first time), else why not. fn usable(&self, start: bool) -> Result<&lightcraft_denoise::remote::Remote, String> { - let greeted = if start { Some(self.greeted.get_or_init(|| self.runner.hello().map_err(|e| e.to_string()))) } else { self.greeted.get() }; + let greeted = if start { Some(self.greeted.get_or_init(|| self.greet())) } else { self.greeted.get() }; match greeted { None => Err("not contacted yet".into()), Some(Err(why)) => Err(why.clone()), @@ -194,12 +219,28 @@ impl RemoteSide { } } + /// The server's time for a tile, once greeted. + fn tile_ms(&self) -> Option { + self.greeted.get().and_then(|g| g.as_ref().ok()).map(|g| g.1) + } + fn describe(&self) -> Value { match self.greeted.get() { None => json!({"address": self.runner.address(), "state": "pending"}), Some(Err(why)) => json!({"address": self.runner.address(), "state": "unavailable", "reason": why}), - Some(Ok(info)) => { - let mut v = json!({"address": info.address, "device": info.device, "state": if self.runner.healthy() { "used" } else { "given up" }, "failures": self.runner.failures()}); + Some(Ok((info, ms))) => { + let slower = *self.slower.lock().unwrap_or_else(PoisonError::into_inner); + let state = match () { + _ if !self.runner.healthy() => "given up", + _ if slower.is_some() => "slower", + _ if self.share.load(Ordering::Relaxed) => "used", + _ => "idle", + }; + let mut v = + json!({"address": info.address, "device": info.device, "state": state, "remoteMs": ms, "failures": self.runner.failures()}); + if let (Some((_, local)), Some(o)) = (slower, v.as_object_mut()) { + o.insert("localMs".into(), json!(local)); + } if let (Some(e), Some(o)) = (self.runner.last_error(), v.as_object_mut()) { o.insert("lastError".into(), json!(e)); } @@ -393,7 +434,7 @@ impl Network { #[cfg(feature = "denoise")] impl TileRunner for Network { fn run(&self, input: &[f32]) -> Result, RunError> { - if let Some(Ok(remote)) = self.remote.as_ref().map(|r| r.usable(false)) + if let Some(Ok(remote)) = self.remote.as_ref().filter(|r| r.share.load(Ordering::Relaxed)).map(|r| r.usable(false)) && let Some(Ok(out)) = remote.try_run(input) { return Ok(out); @@ -425,12 +466,32 @@ impl Model for Network { Err(_) => (false, threads), }; self.local_card.store(card, Ordering::Relaxed); - match self.remote.as_ref().map(|r| r.usable(true)) { - // the server's slots come on top of this computer's own tiles at once - Some(Ok(remote)) => (self, local + remote.slots()), - _ if card => (self, local), - _ => (&self.cpu, local), + if let Some(r) = &self.remote { + // this computer's time for a tile: the card's, or the CPU's with `threads` tiles sharing it (as + // `card_is_faster` counts them); unknown when the card was never set up (then the server is used) + let local_ms = match self.gpu.get() { + Some(Ok(g)) if card => g.card_ms, + Some(Ok(g)) => g.cpu_ms / (1.0 + 0.5 * threads.saturating_sub(1) as f64), + _ => f64::INFINITY, + }; + let share = match (r.usable(true), r.tile_ms()) { + (Ok(_), Some(ms)) if ms < local_ms => { + *r.slower.lock().unwrap_or_else(PoisonError::into_inner) = None; + true + } + (Ok(_), Some(ms)) => { + *r.slower.lock().unwrap_or_else(PoisonError::into_inner) = Some((ms, local_ms)); + false + } + _ => false, + }; + r.share.store(share, Ordering::Relaxed); + if let (true, Ok(remote)) = (share, r.usable(false)) { + // the server's slots come on top of this computer's own tiles at once + return (self, local + remote.slots()); + } } + if card { (self, local) } else { (&self.cpu, local) } } fn self_test(&self, run_on: RunOn) -> Result { @@ -1660,10 +1721,7 @@ mod gpu_tests { overlap: 16, gain: Gain::MatchMean { nominal: 1.0, max_deviation: 0.05 }, }; - let remote = RemoteSide { - runner: lightcraft_denoise::remote::Remote::new(addr, "remote-test-token-0123", sha256, tile as usize), - greeted: OnceLock::new(), - }; + let remote = RemoteSide::new(lightcraft_denoise::remote::Remote::new(addr, "remote-test-token-0123", sha256, tile as usize)); network(path, &manifest, Some(remote)).unwrap() } @@ -1682,6 +1740,15 @@ mod gpu_tests { let d = net.device(RunOn::Cpu); assert_eq!(d["remote"]["state"], "used", "{d}"); assert_eq!(d["remote"]["device"], "CPU (test server)"); + // a local card faster than the server there and back (here a stand-in that takes no time): nothing is shared, + // the photo never waits for the slower server, and the status says why + let mut net = with_remote(&path, 64, &addr, &sha); + net.gpu = OnceLock::from(Ok(GpuSide { runner: Box::new(cpu.clone()), adapter: "Fast card".into(), card_ms: 1e-6, cpu_ms: 1000.0 })); + let (_, at_once) = net.runner(RunOn::Gpu, 8); + assert_eq!(at_once, GPU_PARALLEL, "no server slots"); + let d = net.device(RunOn::Gpu); + assert_eq!(d["remote"]["state"], "slower", "{d}"); + assert!(d["remote"]["remoteMs"].as_f64().is_some_and(|ms| ms > 0.0) && d["remote"]["localMs"].as_f64().is_some(), "{d}"); // a server running another model is refused: everything runs here let other = server(lightcraft_denoise::runtime::CpuRunner::load(&path, &cpu.manifest().clone()).unwrap(), "0000", 64); let net = with_remote(&path, 64, &other, &sha); diff --git a/docs/denoise.md b/docs/denoise.md index b5a171767..926b1bee7 100644 --- a/docs/denoise.md +++ b/docs/denoise.md @@ -138,9 +138,14 @@ export LIGHTCRAFT_DENOISE_URL=100.101.102.103:7990 export LIGHTCRAFT_DENOISE_TOKEN='…the same token…' ``` -The server's slots (four tiles in flight) are added to the laptop's own: a tile goes to the server when one of its -slots is free and runs locally otherwise, so a server behind a slow link still adds speed instead of costing it. A -tile the server fails runs locally; after three failures the server is no longer used for that model. Both ends must +When the laptop first uses the model it greets the server and times a check tile there and back. If that is faster +than a tile on the laptop (its graphics card, or its processor with the tiles it runs at once), the server's slots +(four tiles in flight) are added to the laptop's own: a tile goes to the server when one of its slots is free and runs +locally otherwise. If it is slower, nothing is sent, so a photo never waits for the slower machine: +`denoise.status` → `device.remote.state` is `slower`, with both times (`remoteMs`, `localMs`). Measured with +RawNIND: an Apple M3 laptop does a tile in 116 ms, an RTX 3060 Ti reached over Tailscale 345 ms there and back, +so that laptop keeps its tiles; one without a fast card hands them over. A tile the server fails runs locally; after +three failures the server is no longer used for that model. Both ends must run the same model file: the client compares the manifest's SHA-256 (or the file's) when it first greets the server, and the server checks it again for every tile. `denoise.status` → `device.remote` reports the server's address, its device and whether it is used, or why not.