T10.6: drop RGBA payload from the wire when the hardware path publishes a surface
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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<RenderImage>");
|
||||
let second_image = second
|
||||
.image
|
||||
.as_ref()
|
||||
.expect("software path always produces an Arc<RenderImage>");
|
||||
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<RenderImage> 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),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<RenderImage>,
|
||||
/// 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<Arc<RenderImage>>,
|
||||
/// 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<CVPixelBuffer>,
|
||||
}
|
||||
@@ -100,8 +103,21 @@ impl WebSurfaceFrame {
|
||||
}
|
||||
|
||||
fn from_parts(parts: WebSurfaceFrameParts) -> Result<Self, WebSurfaceError> {
|
||||
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,
|
||||
|
||||
@@ -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<RenderImage>.
|
||||
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(
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -129,6 +129,10 @@ fn run_live_bench() -> Result<(), Box<dyn Error>> {
|
||||
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<dyn Error>> {
|
||||
!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<dyn Error>> {
|
||||
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<FramePerfSummary>,
|
||||
surface_handles: Vec<BenchSurfaceHandle>,
|
||||
current_surface_ids: Vec<u64>,
|
||||
rgba_bytes_received: u64,
|
||||
}
|
||||
|
||||
fn spawn_sidecar(kind: &str, profile_data_dir: &PathBuf) -> Result<Child, Box<dyn Error>> {
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user