From 7f3b8b42b3d493f72c360cda3b3422e00973dec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 10 May 2026 19:36:08 -0400 Subject: [PATCH] Dedup identical RGBA payloads against the last frame's Arc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebSurfaceFrame::from_parts was unconditionally calling Arc::new(RenderImage::new([image::Frame::new(image_buffer)])) on every live frame, even when the underlying bytes were byte-for-byte identical to the previous frame. At 60 fps on a 1080p canvas that was ~960 MB/s of host-side cloning plus a fresh GPUI texture allocation on every tick — the bottleneck Linus + Karpathy + Jony flagged as the next material step after dropping the file-system pixel pipe (a80d039). A thread_local single-slot cache in web_surface_frame.rs now keys on a 64-bit DefaultHasher of the raw RGBA bytes. On a cache hit the existing Arc is reused; on a miss the buffer is built once, stored, and returned. Steady-state idle pages stop churning the GPUI texture pool entirely. Hash collisions are 1 in 2^64 — if that ever becomes a real worry, the cache key can be widened to length + a sample of bytes before paying the full memcmp; not worth doing today. The T10 red guard (identical_live_frames_share_render_image_arc) drops its #[ignore] attribute outright per the contract it documented. The T7 red guard (user_click_in_rendered_web_canvas_reaches_input_pipeline) remains ignored — it's a separate diagnosis tracked under T13. cargo test --bin ely_app: 113 passed, 0 failed, 1 ignored (was 112+2, T10 guard went green). cargo test --bin ely_app -- --ignored: 1 failed (only T7 click pipeline remains red). Follow-ups (left for the endgame T10 IOSurface path): * a per-tab cache would prevent multi-tab switching from thrashing the single slot; defer until a real multi-tab scroll benchmark shows it matters. * the endgame is OffscreenRenderingContext + IOSurface so the GPU texture itself is the source of truth and the host-side Vec + ImageBuffer + RenderImage allocation chain disappears entirely. --- .../ely_app/src/shell/gpui_harness_tests.rs | 19 ++---- crates/ely_app/src/shell/web_surface_frame.rs | 60 ++++++++++++++++--- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/crates/ely_app/src/shell/gpui_harness_tests.rs b/crates/ely_app/src/shell/gpui_harness_tests.rs index d4bf81c..7b54dc3 100644 --- a/crates/ely_app/src/shell/gpui_harness_tests.rs +++ b/crates/ely_app/src/shell/gpui_harness_tests.rs @@ -271,20 +271,13 @@ async fn baseline_overlay_div_receives_simulated_click(cx: &mut TestAppContext) /// against the last bytes or switch to `OffscreenRenderingContext` + /// IOSurface so the GPU texture is the source of truth. /// -/// `#[ignore]` so default `cargo test` stays green; remove the -/// attribute the moment T10 lands so the regression is permanent. +/// Regression guard: with the single-slot `LAST_FRAME_IMAGE` cache in +/// `web_surface_frame.rs`, two `ServoLiveFrame` inputs carrying +/// byte-identical RGBA payloads now share the same `Arc`. +/// Without this guard a regression that drops the cache silently +/// returns to ~960 MB/s of host-side RGBA cloning + per-frame GPUI +/// texture allocations. #[test] -#[ignore = "T10 red guard. Failure mode confirmed via `cargo test -- --ignored`: \ - two ServoLiveFrames carrying byte-identical RGBA payloads still \ - produce distinct Arc instances, because \ - WebSurfaceFrame::from_parts unconditionally calls \ - Arc::new(RenderImage::new(...)). At 60 fps 1080p that is \ - ~960 MB/s of host-side RGBA cloning + GPUI texture allocations \ - and is the next bottleneck after the file-system pipe. The \ - fix lives in WebSurfaceFrame (interim: dedup upload against \ - last frame bytes) or in the rendering pipeline (final: \ - OffscreenRenderingContext + IOSurface zero-copy). The fix \ - commit must remove this attribute outright — not toggle it."] fn identical_live_frames_share_render_image_arc() { let width = 16u32; let height = 8u32; diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 533041b..35bdb6a 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -1,3 +1,6 @@ +use std::cell::RefCell; +use std::collections::hash_map::DefaultHasher; +use std::hash::Hasher; use std::sync::Arc; use gpui::RenderImage; @@ -6,6 +9,21 @@ use thiserror::Error; use crate::services::servo_live::ServoLiveFrame; +thread_local! { + /// Single-slot cache of the most-recently-uploaded RGBA payload. + /// At 60 fps on a 1080p canvas the previous `from_parts` was + /// unconditionally building a fresh `Arc` for every + /// tick — even when the bytes were bit-identical to the last + /// frame. The cache keys on a 64-bit hash of the raw bytes and + /// reuses the existing `Arc` whenever the hash + /// matches, so steady-state idle pages no longer churn the GPUI + /// texture pool. Hash collisions are 1 in 2^64; if they ever + /// matter we'll trade in length + first/last 32 bytes as a + /// disambiguator before paying the full memcmp. + static LAST_FRAME_IMAGE: RefCell)>> = + const { RefCell::new(None) }; +} + #[cfg(all(test, feature = "live-site-smoke"))] use super::web_surface_geometry::WebSurfaceSize; use super::web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollOffset}; @@ -61,14 +79,8 @@ impl WebSurfaceFrame { } fn from_parts(parts: WebSurfaceFrameParts) -> Result { - let Some(image_buffer) = - ImageBuffer::, _>::from_raw(parts.width, parts.height, parts.rgba_bytes) - else { - return Err(WebSurfaceError::InvalidFrameBuffer { - width: parts.width, - height: parts.height, - }); - }; + let bytes_hash = rgba_hash(&parts.rgba_bytes); + let image = resolve_render_image(parts.width, parts.height, parts.rgba_bytes, bytes_hash)?; Ok(Self { requested_url: parts.requested_url, @@ -87,7 +99,7 @@ impl WebSurfaceFrame { content_pixel_count: parts.content_pixel_count, #[cfg(all(test, feature = "live-site-smoke"))] sample_hash: parts.sample_hash, - image: Arc::new(RenderImage::new([image::Frame::new(image_buffer)])), + image, }) } @@ -187,3 +199,33 @@ pub(super) enum WebSurfaceError { #[error("invalid servo frame buffer for {width}x{height}")] InvalidFrameBuffer { width: u32, height: u32 }, } + +fn rgba_hash(bytes: &[u8]) -> u64 { + let mut hasher = DefaultHasher::new(); + hasher.write(bytes); + hasher.finish() +} + +fn resolve_render_image( + width: u32, + height: u32, + rgba_bytes: Vec, + bytes_hash: u64, +) -> Result, WebSurfaceError> { + LAST_FRAME_IMAGE.with( + |cache| -> Result, WebSurfaceError> { + let mut cache = cache.borrow_mut(); + if let Some((cached_hash, cached_image)) = cache.as_ref() { + if *cached_hash == bytes_hash { + return Ok(cached_image.clone()); + } + } + + let image_buffer = ImageBuffer::, _>::from_raw(width, height, rgba_bytes) + .ok_or(WebSurfaceError::InvalidFrameBuffer { width, height })?; + let new_image = Arc::new(RenderImage::new([image::Frame::new(image_buffer)])); + *cache = Some((bytes_hash, new_image.clone())); + Ok(new_image) + }, + ) +}