From 6e2bd20a9bd3dd9d42a4f4f6e3b8cccc21db15d2 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 23:52:12 -0400 Subject: [PATCH] T10.6: drop RGBA payload from the wire when the hardware path publishes a surface --- crates/ely_app/src/services/servo_live.rs | 27 +++++++++------- .../ely_app/src/shell/gpui_harness_tests.rs | 14 +++++++-- crates/ely_app/src/shell/web_surface_frame.rs | 26 +++++++++++++--- crates/ely_app/src/shell/web_surface_view.rs | 26 +++++++++++----- .../src/bin/ely_servo_sidecar/live.rs | 17 ++++++++-- .../ely_servo_host/tests/live_perf_bench.rs | 31 ++++++++++++++++++- 6 files changed, 111 insertions(+), 30 deletions(-) diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index 9b44d9e..02701d9 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -132,13 +132,14 @@ impl ServoLiveClient { // Sanity bound the byte count advertised by the sidecar // header so a buggy or hostile sidecar can't park us on // `read_exact` for an arbitrarily-sized buffer. The honest - // upper limit is `width * height * 4` (RGBA8); anything - // larger is a protocol violation and we fail the request - // instead of allocating against it. + // upper limit is `width * height * 4` (RGBA8); `0` is the + // explicit "hardware path active, sample the IOSurface + // instead" signal — anything else is a protocol violation. let pixel_byte_count = (report.width as u64) .saturating_mul(report.height as u64) .saturating_mul(4); - if (report.rgba_byte_count as u64) != pixel_byte_count { + let advertised = report.rgba_byte_count as u64; + if advertised != 0 && advertised != pixel_byte_count { return Err(ServoLiveError::FrameBudgetExceeded { advertised: report.rgba_byte_count, pixel_budget: pixel_byte_count, @@ -147,14 +148,18 @@ impl ServoLiveClient { }); } - // Raw frame bytes follow the JSON header on the same pipe. - // `read_exact` drains BufReader's buffer first (the line read - // never crosses the `\n` boundary) and then pulls the rest - // straight from the child's stdout — no fs::read, no temp file. + // Raw frame bytes follow the JSON header on the same pipe + // ONLY when the sidecar didn't drop the payload for the + // hardware path. `read_exact` drains BufReader's buffer first + // (the line read never crosses the `\n` boundary) and then + // pulls the rest straight from the child's stdout — no + // fs::read, no temp file. let mut rgba_bytes = vec![0u8; report.rgba_byte_count]; - self.stdout - .read_exact(&mut rgba_bytes) - .map_err(ServoLiveError::FrameRead)?; + if report.rgba_byte_count > 0 { + self.stdout + .read_exact(&mut rgba_bytes) + .map_err(ServoLiveError::FrameRead)?; + } let mut frame = ServoLiveFrame::from_parts(report, rgba_bytes); diff --git a/crates/ely_app/src/shell/gpui_harness_tests.rs b/crates/ely_app/src/shell/gpui_harness_tests.rs index b8bd0b6..7665a23 100644 --- a/crates/ely_app/src/shell/gpui_harness_tests.rs +++ b/crates/ely_app/src/shell/gpui_harness_tests.rs @@ -1085,16 +1085,24 @@ fn identical_live_frames_share_render_image_arc() { ) .expect("second frame builds from identical bytes"); + let first_image = first + .image + .as_ref() + .expect("software path always produces an Arc"); + let second_image = second + .image + .as_ref() + .expect("software path always produces an Arc"); assert!( - Arc::ptr_eq(&first.image, &second.image), + Arc::ptr_eq(first_image, second_image), "TDD red: two ServoLiveFrames with byte-identical RGBA produced \ distinct Arc instances (first={:p}, second={:p}). \ WebSurfaceFrame::from_parts must dedup the upload against the \ previous frame's bytes, or the rendering pipeline must switch \ to a GPU-side source of truth (IOSurface) so per-frame host \ allocations stop entirely.", - Arc::as_ptr(&first.image), - Arc::as_ptr(&second.image), + Arc::as_ptr(first_image), + Arc::as_ptr(second_image), ); } diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 3f8950f..6466c0f 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -57,12 +57,15 @@ pub(super) struct WebSurfaceFrame { content_pixel_count: u64, #[cfg(all(test, feature = "live-site-smoke"))] sample_hash: u64, - pub(super) image: Arc, + /// Software-path image. `None` whenever the sidecar took the + /// hardware shortcut and dropped the RGBA payload from the + /// wire — the IOSurface in `pixel_buffer` is the source of truth + /// for that frame. + pub(super) image: Option>, /// Hardware-path companion: when present, the view samples the /// IOSurface through GPUI's Metal pipeline via `gpui::surface(...)` /// instead of uploading the RGBA bytes again. Always `None` on - /// the software path; the RGBA copy in `image` is the source of - /// truth in that case. + /// the software path; `image` is the source of truth there. #[cfg(target_os = "macos")] pub(super) pixel_buffer: Option, } @@ -100,8 +103,21 @@ impl WebSurfaceFrame { } fn from_parts(parts: WebSurfaceFrameParts) -> Result { - let bytes_hash = rgba_hash(&parts.rgba_bytes); - let image = resolve_render_image(parts.width, parts.height, parts.rgba_bytes, bytes_hash)?; + // Hardware path frames arrive with `rgba_bytes` empty — the + // sidecar dropped the 8 MB payload from the wire and the + // receiver samples the IOSurface directly. In that case the + // RGBA hash + LAST_FRAME_IMAGE dedup are skipped entirely. + let image = if parts.rgba_bytes.is_empty() { + None + } else { + let bytes_hash = rgba_hash(&parts.rgba_bytes); + Some(resolve_render_image( + parts.width, + parts.height, + parts.rgba_bytes, + bytes_hash, + )?) + }; Ok(Self { requested_url: parts.requested_url, diff --git a/crates/ely_app/src/shell/web_surface_view.rs b/crates/ely_app/src/shell/web_surface_view.rs index 3bfff4e..53a1dd6 100644 --- a/crates/ely_app/src/shell/web_surface_view.rs +++ b/crates/ely_app/src/shell/web_surface_view.rs @@ -18,9 +18,10 @@ pub(super) fn render_ready_web_surface( // IOSurface and we successfully imported it into a CVPixelBuffer. // GPUI's `surface(...)` hands the buffer to its Blade Metal // renderer, which samples the IOSurface directly — no - // RGBA→texture upload, no LAST_FRAME_IMAGE dedup needed. Falls - // back to the software RGBA image when the buffer is missing - // (software webview, import failure, non-macOS host). + // RGBA→texture upload, no LAST_FRAME_IMAGE dedup needed. The + // sidecar drops the RGBA payload from the wire whenever it + // populates `pixel_buffer`, so `image` is `None` on this branch + // and there's no software memcpy to fall back on. #[cfg(target_os = "macos")] if let Some(pixel_buffer) = frame.pixel_buffer.as_ref() { return render_web_surface( @@ -29,11 +30,20 @@ pub(super) fn render_ready_web_surface( surface(pixel_buffer.clone()).size_full().object_fit(ObjectFit::Fill), ); } - render_web_surface( - tab, - state_entity, - img(ImageSource::Render(frame.image.clone())).size_full().object_fit(ObjectFit::Fill), - ) + // Software path: the sidecar streamed the RGBA payload and the + // receiver decoded it into an Arc. + if let Some(image) = frame.image.as_ref() { + return render_web_surface( + tab, + state_entity, + img(ImageSource::Render(image.clone())).size_full().object_fit(ObjectFit::Fill), + ); + } + // Both image variants empty: shouldn't happen because the sidecar + // only drops RGBA when it has a hardware surface to publish, but + // a blank canvas is the honest user-facing fallback if it ever + // does. + render_web_surface(tab, state_entity, div().size_full()) } pub(super) fn render_loading_web_surface( diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs index 0171c53..296147a 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs @@ -225,11 +225,24 @@ fn write_outcome( if let Some(summary) = pending_summary.take() { outcome.response.perf = Some(summary); } + // Hardware path: receiver samples the IOSurface directly through + // its CVPixelBuffer cache, so the raw RGBA payload is dead + // weight. Drop it from the wire (and zero the byte count in the + // header so the client knows nothing follows). At 1080p × 60 fps + // that's 8 MB × 60 = ~480 MB/s of pipe traffic eliminated. + let drop_rgba_payload = outcome.response.current_surface_id.is_some(); + if drop_rgba_payload { + if let Some(report) = outcome.response.frame.as_mut() { + report.rgba_byte_count = 0; + } + } let write_started_at = Instant::now(); serde_json::to_writer(&mut *stdout, &outcome.response)?; stdout.write_all(b"\n")?; - if let Some(frame) = outcome.frame.as_ref() { - stdout.write_all(frame.rgba_bytes())?; + if !drop_rgba_payload { + if let Some(frame) = outcome.frame.as_ref() { + stdout.write_all(frame.rgba_bytes())?; + } } stdout.flush()?; if frame_present { diff --git a/crates/ely_servo_host/tests/live_perf_bench.rs b/crates/ely_servo_host/tests/live_perf_bench.rs index 5907990..024bb8d 100644 --- a/crates/ely_servo_host/tests/live_perf_bench.rs +++ b/crates/ely_servo_host/tests/live_perf_bench.rs @@ -129,6 +129,10 @@ fn run_live_bench() -> Result<(), Box> { print_summaries(&kind, frames, &outcome.summaries); print_surface_handles(&kind, &outcome.surface_handles); print_current_surface_summary(&kind, &outcome.current_surface_ids); + eprintln!( + "\n=== ELY_PERF_KIND={kind} rgba_bytes_received={} ===", + outcome.rgba_bytes_received + ); assert!( !outcome.summaries.is_empty(), "expected at least one FramePerfSummary across {frames} frames" @@ -154,6 +158,18 @@ fn run_live_bench() -> Result<(), Box> { !outcome.current_surface_ids.is_empty(), "hardware path must report current_surface_id on every frame" ); + // T10.6: once the receiver samples the IOSurface directly, + // the sidecar drops the RGBA payload. The initial navigate + // response may still carry bytes (no current_surface_id yet + // because surfman hasn't bound the painted surface), but the + // steady-state per-frame cost must be zero. + assert!( + outcome.rgba_bytes_received < (frames as u64) * 1_024, + "hardware path leaked {} RGBA bytes across {} frames \ + (expected ~0 — the wire-drop optimisation regressed)", + outcome.rgba_bytes_received, + frames + 1, + ); } else { assert!( outcome.surface_handles.is_empty(), @@ -163,6 +179,15 @@ fn run_live_bench() -> Result<(), Box> { outcome.current_surface_ids.is_empty(), "software path must never report current_surface_id" ); + // Software path keeps streaming pixels — every frame must + // carry a full RGBA payload. + let viewport_bytes = (1024u64) * (768u64) * 4; + assert!( + outcome.rgba_bytes_received >= viewport_bytes, + "software path delivered only {} bytes — expected at least one full frame ({})", + outcome.rgba_bytes_received, + viewport_bytes, + ); } Ok(()) } @@ -171,6 +196,7 @@ struct BenchOutcome { summaries: Vec, surface_handles: Vec, current_surface_ids: Vec, + rgba_bytes_received: u64, } fn spawn_sidecar(kind: &str, profile_data_dir: &PathBuf) -> Result> { @@ -199,6 +225,7 @@ fn drive_bench( let mut summaries = Vec::new(); let mut surface_handles = Vec::new(); let mut current_surface_ids = Vec::new(); + let mut rgba_bytes_received: u64 = 0; let navigate = build_ensure(tab, profile_id, url, 0, 0, false); write_request(stdin, &navigate)?; @@ -206,6 +233,7 @@ fn drive_bench( record_summary(&response, kind, &mut summaries); record_surface_handle(&response, kind, &mut surface_handles); record_current_surface_id(&response, &mut current_surface_ids); + rgba_bytes_received += response.frame.as_ref().map_or(0, |f| f.rgba_byte_count as u64); let mut accumulated_scroll = 0; for frame_index in 0..frames { @@ -224,10 +252,11 @@ fn drive_bench( record_summary(&response, kind, &mut summaries); record_surface_handle(&response, kind, &mut surface_handles); record_current_surface_id(&response, &mut current_surface_ids); + rgba_bytes_received += response.frame.as_ref().map_or(0, |f| f.rgba_byte_count as u64); } let _ = accumulated_scroll; - Ok(BenchOutcome { summaries, surface_handles, current_surface_ids }) + Ok(BenchOutcome { summaries, surface_handles, current_surface_ids, rgba_bytes_received }) } fn record_surface_handle(