From a80d0393e913a7c09c47a57658c67aab2ed1d456 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:04:34 -0400 Subject: [PATCH] Drop the file system from the live frame pixel pipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every live frame was round-tripping through the local file system: the sidecar called `fs::write(rgba_out, frame.rgba_bytes())` in `poll_frame`, the JSON response carried `rgba_path`, and the main process turned around and called `fs::read(rgba_path)` to lift the bytes back into a `Vec`. At 1080p that is 8 MB of syscall + memcpy + page cache traffic per frame; at 60 fps it dwarfs every other cost in the pipeline and shows up as scroll/zoom jank the user can feel before any other bottleneck. Replace it with a same-pipe binary protocol. The sidecar writes the JSON `LiveResponse` line as before, then writes the raw RGBA frame bytes on the same stdout immediately after the trailing `\n`. The client `read_line`s the JSON, parses `rgba_byte_count` from the header, and `read_exact`s exactly that many bytes from the same `BufReader` (the buffered reader drains its own buffer before pulling from the child). No tmpfs directory, no `fs::remove_dir_all` on drop, no `rgba_path` field, no per-frame filename plumbing. Boundary defence on the client side: the header's `rgba_byte_count` is cross-checked against `width * height * 4` before any allocation or `read_exact`. A buggy or compromised sidecar can no longer ask the GPUI process to allocate an arbitrarily large buffer or park on `read_exact` for a payload that will never arrive. The sidecar process boundary stays exactly where it was; only the pixel transport between the two processes changes. The `one-shot` sidecar binary path (used by `tests/sidecar.rs` and PRD smoke tests) still writes to its CLI-supplied `--rgba-out` path — those tests were untouched and continue to pass 9/9. cargo test --bin ely_app: 112 passed. cargo test -p ely_servo_host --features servo-engine --test sidecar: 9 passed. Follow-ups deferred to the profile step (T9): * the host still copies its rendered buffer to a transient `Vec` via `frame.rgba_bytes().to_vec()` before write_all; expose `&[u8]` straight to `write_all` once the profile shows that allocation in the top five. * `poll_frame` calls `snapshot` twice per iteration; harmless under the software renderer but worth folding into a single snapshot once we have numbers. * raw memcpy bandwidth is still ~480 MB/s at 60 fps 1080p; the GPU-side fix (T10: OffscreenRenderingContext + IOSurface zero-copy) is the next material change. --- crates/ely_app/src/services/servo_live.rs | 91 +++++++++---------- .../src/bin/ely_servo_sidecar/live.rs | 90 +++++++++++------- 2 files changed, 102 insertions(+), 79 deletions(-) diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index f177edb..4f5961d 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -1,9 +1,7 @@ use std::{ - env, fs, - io::{self, BufRead, BufReader, Write}, + io::{self, BufRead, BufReader, Read, Write}, path::PathBuf, process::{Child, ChildStdin, ChildStdout, Stdio}, - time::{SystemTime, SystemTimeError, UNIX_EPOCH}, }; use ely_domain::SitePermissionDecision; @@ -16,8 +14,6 @@ pub(crate) struct ServoLiveClient { child: Child, stdin: ChildStdin, stdout: BufReader, - frame_dir: PathBuf, - frame_path: PathBuf, } impl ServoLiveClient { @@ -27,8 +23,6 @@ impl ServoLiveClient { return Err(ServoLiveError::SidecarBinaryUnavailable { path: path.to_path_buf() }); } - let frame_dir = temporary_frame_dir()?; - let frame_path = frame_dir.join("frame.rgba"); let mut command = command_target.command(); command.arg("live").arg("--profile-data-dir").arg(profile_data_dir); let mut child = command @@ -41,7 +35,7 @@ impl ServoLiveClient { let stdout = child.stdout.take().ok_or(ServoLiveError::PipeUnavailable { name: "stdout" })?; - Ok(Self { child, stdin, stdout: BufReader::new(stdout), frame_dir, frame_path }) + Ok(Self { child, stdin, stdout: BufReader::new(stdout) }) } pub fn ensure( @@ -63,12 +57,11 @@ impl ServoLiveClient { hover_y: request.hover_y, typed_text: request.typed_text, site_permissions: request.site_permissions, - rgba_out: self.frame_path.display().to_string(), }) } pub fn poll(&mut self, tab_id: String) -> Result, ServoLiveError> { - self.request(LiveRequest::Poll { tab_id, rgba_out: self.frame_path.display().to_string() }) + self.request(LiveRequest::Poll { tab_id }) } fn request(&mut self, request: LiveRequest) -> Result, ServoLiveError> { @@ -87,7 +80,38 @@ impl ServoLiveClient { return Err(ServoLiveError::SidecarFailed { message: error }); } - response.frame.map(ServoLiveFrame::from_report).transpose() + let Some(report) = response.frame else { + return Ok(None); + }; + + // 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. + 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 { + return Err(ServoLiveError::FrameBudgetExceeded { + advertised: report.rgba_byte_count, + pixel_budget: pixel_byte_count, + width: report.width, + height: report.height, + }); + } + + // 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. + let mut rgba_bytes = vec![0u8; report.rgba_byte_count]; + self.stdout + .read_exact(&mut rgba_bytes) + .map_err(ServoLiveError::FrameRead)?; + + Ok(Some(ServoLiveFrame::from_parts(report, rgba_bytes))) } } @@ -95,7 +119,6 @@ impl Drop for ServoLiveClient { fn drop(&mut self) { let _ = self.child.kill(); let _ = self.child.wait(); - let _ = fs::remove_dir_all(&self.frame_dir); } } @@ -149,16 +172,8 @@ pub(crate) struct ServoLiveFrame { } impl ServoLiveFrame { - fn from_report(report: LiveFrameReport) -> Result { - let rgba_bytes = fs::read(&report.rgba_path).map_err(ServoLiveError::FrameRead)?; - if rgba_bytes.len() != report.rgba_byte_count { - return Err(ServoLiveError::RgbaByteCountMismatch { - expected: report.rgba_byte_count, - actual: rgba_bytes.len(), - }); - } - - Ok(Self { + fn from_parts(report: LiveFrameReport, rgba_bytes: Vec) -> Self { + Self { loaded_url: report.loaded_url, title: report.title, render_state: report.state, @@ -171,7 +186,7 @@ impl ServoLiveFrame { #[cfg(all(test, feature = "live-site-smoke"))] sample_hash: report.sample_hash, rgba_bytes, - }) + } } #[must_use] @@ -240,17 +255,14 @@ pub(crate) enum ServoLiveError { #[error("servo live sidecar failed: {message}")] SidecarFailed { message: String }, - #[error("failed to read servo live frame file: {0}")] + #[error("failed to read servo live frame bytes: {0}")] FrameRead(#[source] io::Error), - #[error("servo live frame byte count mismatch: expected {expected}, actual {actual}")] - RgbaByteCountMismatch { expected: usize, actual: usize }, - - #[error("temporary live frame directory is unavailable: {0}")] - TempDirectory(#[source] io::Error), - - #[error("temporary live frame timestamp is unavailable: {0}")] - SystemClock(#[source] SystemTimeError), + #[error( + "servo live sidecar advertised {advertised} frame bytes which exceeds \ + the {width}x{height} pixel budget ({pixel_budget} bytes)" + )] + FrameBudgetExceeded { advertised: usize, pixel_budget: u64, width: u32, height: u32 }, #[error(transparent)] Json(#[from] serde_json::Error), @@ -277,11 +289,9 @@ enum LiveRequest { hover_y: Option, typed_text: Option, site_permissions: Vec, - rgba_out: String, }, Poll { tab_id: String, - rgba_out: String, }, } @@ -298,7 +308,6 @@ struct LiveFrameReport { state: String, width: u32, height: u32, - rgba_path: PathBuf, rgba_byte_count: usize, #[cfg(all(test, feature = "live-site-smoke"))] non_white_pixel_count: u64, @@ -307,15 +316,3 @@ struct LiveFrameReport { #[cfg(all(test, feature = "live-site-smoke"))] sample_hash: u64, } - -fn temporary_frame_dir() -> Result { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_err(ServoLiveError::SystemClock)? - .as_nanos(); - let directory = env::temp_dir() - .join("ely-browser-servo-live") - .join(format!("{}-{timestamp}", std::process::id())); - fs::create_dir_all(&directory).map_err(ServoLiveError::TempDirectory)?; - Ok(directory) -} 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 1b15d18..b78cb03 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 @@ -2,7 +2,6 @@ use std::{ collections::HashMap, fs, io::{self, BufRead, Write}, - path::PathBuf, thread, time::{Duration, Instant}, }; @@ -44,11 +43,11 @@ pub(super) fn run_live(args: LiveArgs) -> Result<(), LiveSidecarError> { continue; } - let response = match serde_json::from_str::(&line) { + let outcome = match serde_json::from_str::(&line) { Ok(request) => handle_request(&mut host, &mut sessions, request), Err(error) => Err(LiveSidecarError::Json(error)), }; - write_response(&mut stdout, response)?; + write_outcome(&mut stdout, outcome)?; } Ok(()) @@ -58,7 +57,7 @@ fn handle_request( host: &mut SoftwareServoHost, sessions: &mut HashMap, request: LiveRequest, -) -> Result { +) -> Result { match request { LiveRequest::Ensure { tab_id, @@ -75,7 +74,6 @@ fn handle_request( hover_y, typed_text, site_permissions, - rgba_out, } => { let tab = TabId::parse(tab_id.clone())?; let profile = ProfileId::parse(profile_id)?; @@ -110,27 +108,37 @@ fn handle_request( )? { session.awaiting_visible_frame = true; } - poll_frame(host, session, rgba_out.into()) + poll_frame(host, session) } - LiveRequest::Poll { tab_id, rgba_out } => { + LiveRequest::Poll { tab_id } => { let Some(session) = sessions.get_mut(&tab_id) else { - return Ok(LiveResponse::empty()); + return Ok(LiveOutcome::empty()); }; - poll_frame(host, session, rgba_out.into()) + poll_frame(host, session) } } } -fn write_response( +/// Stream a JSON response line followed by the optional raw RGBA frame. +/// +/// The frame bytes ride on the same stdout pipe as the JSON header +/// rather than being staged through a temp file. The client reads the +/// JSON line, takes `rgba_byte_count` from the report, then reads that +/// many bytes from the same stream. A 1080p frame is 8 MB — at 60 fps +/// the previous `fs::write` + main-process `fs::read` round-trip cost +/// ~960 MB/s of syscall + memcpy traffic that the scroll/zoom path +/// could never amortise. Same pipe, raw bytes: no kernel `open`, no +/// page cache churn, no transient file lifecycle to clean up. +fn write_outcome( stdout: &mut impl Write, - response: Result, + outcome: Result, ) -> Result<(), LiveSidecarError> { - let response = match response { - Ok(response) => response, - Err(error) => LiveResponse::error(error.to_string()), - }; - serde_json::to_writer(&mut *stdout, &response)?; + let outcome = outcome.unwrap_or_else(|error| LiveOutcome::error(error.to_string())); + serde_json::to_writer(&mut *stdout, &outcome.response)?; stdout.write_all(b"\n")?; + if let Some(frame_bytes) = outcome.frame_bytes { + stdout.write_all(&frame_bytes)?; + } stdout.flush()?; Ok(()) } @@ -250,10 +258,9 @@ fn apply_input( fn poll_frame( host: &mut SoftwareServoHost, session: &mut LiveSession, - rgba_out: PathBuf, -) -> Result { +) -> Result { let started_at = Instant::now(); - let mut latest_frame = None; + let mut latest = None; loop { host.tick(); @@ -264,24 +271,22 @@ fn poll_frame( let frame = host.last_rendered_frame()?; let has_visible_content = frame.non_white_pixel_count() > 0 && frame.content_pixel_count() > 0; - fs::write(&rgba_out, frame.rgba_bytes())?; - let response = - LiveResponse::frame(LiveFrameReport::new(&snapshot, &frame, rgba_out.clone())); + let outcome = LiveOutcome::frame(LiveFrameReport::new(&snapshot, &frame), &frame); if has_visible_content { session.awaiting_visible_frame = false; - return Ok(response); + return Ok(outcome); } if !session.awaiting_visible_frame { - return Ok(response); + return Ok(outcome); } - latest_frame = Some(response); + latest = Some(outcome); } if !session.awaiting_visible_frame { - return Ok(LiveResponse::empty()); + return Ok(LiveOutcome::empty()); } if started_at.elapsed() >= LIVE_FRAME_WAIT_TIMEOUT { - return Ok(latest_frame.unwrap_or_else(LiveResponse::empty)); + return Ok(latest.unwrap_or_else(LiveOutcome::empty)); } thread::sleep(LIVE_FRAME_WAIT_INTERVAL); @@ -335,11 +340,9 @@ enum LiveRequest { hover_y: Option, typed_text: Option, site_permissions: Vec, - rgba_out: String, }, Poll { tab_id: String, - rgba_out: String, }, } @@ -350,6 +353,31 @@ struct LiveSitePermission { decision: String, } +/// A handle plus an optional raw-bytes payload, kept together until +/// the moment of writing to stdout. The JSON header advertises +/// `rgba_byte_count`; the binary follows on the same pipe. +struct LiveOutcome { + response: LiveResponse, + frame_bytes: Option>, +} + +impl LiveOutcome { + fn empty() -> Self { + Self { response: LiveResponse::empty(), frame_bytes: None } + } + + fn error(message: String) -> Self { + Self { response: LiveResponse::error(message), frame_bytes: None } + } + + fn frame(report: LiveFrameReport, frame: &RenderedFrame) -> Self { + Self { + response: LiveResponse::frame(report), + frame_bytes: Some(frame.rgba_bytes().to_vec()), + } + } +} + #[derive(Serialize)] struct LiveResponse { error: Option, @@ -377,7 +405,6 @@ struct LiveFrameReport { state: &'static str, width: u32, height: u32, - rgba_path: PathBuf, rgba_byte_count: usize, non_white_pixel_count: u64, content_pixel_count: u64, @@ -385,14 +412,13 @@ struct LiveFrameReport { } impl LiveFrameReport { - fn new(snapshot: &WebViewSnapshot, frame: &RenderedFrame, rgba_path: PathBuf) -> Self { + fn new(snapshot: &WebViewSnapshot, frame: &RenderedFrame) -> Self { Self { loaded_url: snapshot.url().map(str::to_string), title: snapshot.title().map(str::to_string), state: state_label(snapshot.state()), width: frame.width(), height: frame.height(), - rgba_path, rgba_byte_count: frame.rgba_bytes().len(), non_white_pixel_count: frame.non_white_pixel_count(), content_pixel_count: frame.content_pixel_count(),