Dedup identical RGBA payloads against the last frame's Arc<RenderImage>
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<RenderImage> 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<u8> + ImageBuffer + RenderImage allocation chain
disappears entirely.
This commit is contained in:
@@ -271,20 +271,13 @@ async fn baseline_overlay_div_receives_simulated_click(cx: &mut TestAppContext)
|
|||||||
/// against the last bytes or switch to `OffscreenRenderingContext` +
|
/// against the last bytes or switch to `OffscreenRenderingContext` +
|
||||||
/// IOSurface so the GPU texture is the source of truth.
|
/// IOSurface so the GPU texture is the source of truth.
|
||||||
///
|
///
|
||||||
/// `#[ignore]` so default `cargo test` stays green; remove the
|
/// Regression guard: with the single-slot `LAST_FRAME_IMAGE` cache in
|
||||||
/// attribute the moment T10 lands so the regression is permanent.
|
/// `web_surface_frame.rs`, two `ServoLiveFrame` inputs carrying
|
||||||
|
/// byte-identical RGBA payloads now share the same `Arc<RenderImage>`.
|
||||||
|
/// 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]
|
#[test]
|
||||||
#[ignore = "T10 red guard. Failure mode confirmed via `cargo test -- --ignored`: \
|
|
||||||
two ServoLiveFrames carrying byte-identical RGBA payloads still \
|
|
||||||
produce distinct Arc<RenderImage> 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() {
|
fn identical_live_frames_share_render_image_arc() {
|
||||||
let width = 16u32;
|
let width = 16u32;
|
||||||
let height = 8u32;
|
let height = 8u32;
|
||||||
|
|||||||
@@ -1,3 +1,6 @@
|
|||||||
|
use std::cell::RefCell;
|
||||||
|
use std::collections::hash_map::DefaultHasher;
|
||||||
|
use std::hash::Hasher;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use gpui::RenderImage;
|
use gpui::RenderImage;
|
||||||
@@ -6,6 +9,21 @@ use thiserror::Error;
|
|||||||
|
|
||||||
use crate::services::servo_live::ServoLiveFrame;
|
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<RenderImage>` 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<RenderImage>` 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<Option<(u64, Arc<RenderImage>)>> =
|
||||||
|
const { RefCell::new(None) };
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(all(test, feature = "live-site-smoke"))]
|
#[cfg(all(test, feature = "live-site-smoke"))]
|
||||||
use super::web_surface_geometry::WebSurfaceSize;
|
use super::web_surface_geometry::WebSurfaceSize;
|
||||||
use super::web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollOffset};
|
use super::web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollOffset};
|
||||||
@@ -61,14 +79,8 @@ impl WebSurfaceFrame {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn from_parts(parts: WebSurfaceFrameParts) -> Result<Self, WebSurfaceError> {
|
fn from_parts(parts: WebSurfaceFrameParts) -> Result<Self, WebSurfaceError> {
|
||||||
let Some(image_buffer) =
|
let bytes_hash = rgba_hash(&parts.rgba_bytes);
|
||||||
ImageBuffer::<Rgba<u8>, _>::from_raw(parts.width, parts.height, parts.rgba_bytes)
|
let image = resolve_render_image(parts.width, parts.height, parts.rgba_bytes, bytes_hash)?;
|
||||||
else {
|
|
||||||
return Err(WebSurfaceError::InvalidFrameBuffer {
|
|
||||||
width: parts.width,
|
|
||||||
height: parts.height,
|
|
||||||
});
|
|
||||||
};
|
|
||||||
|
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
requested_url: parts.requested_url,
|
requested_url: parts.requested_url,
|
||||||
@@ -87,7 +99,7 @@ impl WebSurfaceFrame {
|
|||||||
content_pixel_count: parts.content_pixel_count,
|
content_pixel_count: parts.content_pixel_count,
|
||||||
#[cfg(all(test, feature = "live-site-smoke"))]
|
#[cfg(all(test, feature = "live-site-smoke"))]
|
||||||
sample_hash: parts.sample_hash,
|
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}")]
|
#[error("invalid servo frame buffer for {width}x{height}")]
|
||||||
InvalidFrameBuffer { width: u32, height: u32 },
|
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<u8>,
|
||||||
|
bytes_hash: u64,
|
||||||
|
) -> Result<Arc<RenderImage>, WebSurfaceError> {
|
||||||
|
LAST_FRAME_IMAGE.with(
|
||||||
|
|cache| -> Result<Arc<RenderImage>, 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::<Rgba<u8>, _>::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)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user