web browser (Servo)
git clone https://git.lucas.co/cce-browser.git
perf: pace the readback to draws, not to the engine
The two halves of the WPE buffer protocol now say different things at
different times. wpe_view_buffer_rendered — displayed — is said at once, so
the engine's frame pacing never waits on us. wpe_view_buffer_released — the
memory is yours again — waits until the pixels have been copied out, which
happens in pump rather than in the callback. (Saying neither is what stalls
the engine after one frame, which is what the old comment there warned about.)
Holding the buffer buys two things. A frame superseded before anyone read it
goes back unread, so several frames dispatched in one pump's drain cost one
copy instead of N. And pending_draw gates the readback on the chrome having
actually drawn — frame_drawn, called from display_list — so while nothing has
drawn the last frame, the next is left held rather than copied over a picture
no one saw.
The second half is the win, and it is not the one I expected. Measured in a
shadow against a page animating at 63 fps: visible and drawing, 62 of 63
frames are read and the pacing changes nothing, because pump is what
dispatches the engine's frames and does so promptly — the engine never gets
ahead. Minimized, 4 of 64 are read: 60 frames a second handed back unread,
where before an animating page in a hidden window copied its full window size
sixty times a second for nobody. Restoring recovers the full rate inside a
second, with live content and no stale frame.
CCE_BROWSER_FRAME_DEBUG=1 logs produced-against-read once a second, because
the gap is otherwise invisible: a browser skipping nine frames in ten looks
exactly like one copying all ten.
Also drops the reuse argument from read_shm — with the copy moved out of the
callback there is no superseded buffer to refill, and the registry's pool
already hands one back — and releases a held buffer before a tab's view is
destroyed.
29 tests pass; no engine stall across minimize, restore, resize and tab close.
Co-Authored-By: Claude Opus 5 <[email protected]>
CLAUDE.md | 30 ++++++++++++
src/main.rs | 5 ++
src/wpe/host.rs | 130 +++++++++++++++++++++++++++++++++++++++++-----------
src/wpe/subclass.rs | 30 ++++++++----
4 files changed, 160 insertions(+), 35 deletions(-)
diff --git a/CLAUDE.md b/CLAUDE.md
index 352e0aa..5acf4c0 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -152,6 +152,36 @@ explained there. **Do not reintroduce a `Vec` allocation, a swizzle, or a
second copy on this path without measuring** — the numbers above are what each
one costs.
+### The readback is paced to draws
+
+`render_buffer` says the two halves of the buffer protocol at different times,
+and that is the pacing. `wpe_view_buffer_rendered` — *displayed* — is said at
+once, so the engine's own frame pacing never waits on us.
+`wpe_view_buffer_released` — *the memory is yours again* — waits until the
+pixels have been copied out, which happens in `pump`, not in the callback.
+(Saying neither is what stalls the engine after exactly one frame; that is what
+the old comment here warned about.)
+
+Holding the buffer buys two things. A frame superseded before anyone read it is
+handed back **unread**, so several frames dispatched inside one pump's drain
+cost one copy rather than N. And `pending_draw` gates the readback on the
+chrome having actually drawn (`frame_drawn`, called from `display_list`): while
+nothing has drawn the last frame, the next one is left held rather than copied
+over a picture nobody saw.
+
+That second half is where the win is, and it is not the one first expected.
+Measured in a shadow against a page animating at 63 fps: **visible and drawing,
+62 of 63 frames are read — the pacing changes nothing**, because `pump` is what
+dispatches the engine's frames and it dispatches them promptly, so the engine
+never gets ahead. **Minimized, 4 of 64 are read** — 60 handed back unread, a
+page that used to cost its full window size sixty times a second while nobody
+was looking. Restoring recovers the full rate within a second, with live
+content.
+
+`CCE_BROWSER_FRAME_DEBUG=1` logs the two counts once a second; the gap between
+them is invisible from the outside, since a browser that skips nine frames in
+ten looks exactly like one that copies all ten.
+
## Tabs
One `WebView` per tab, all sharing the single rendering context; only the active one
diff --git a/src/main.rs b/src/main.rs
index 1fbbc86..778e297 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -2759,6 +2759,11 @@ impl Application for BrowserApp {
}
fn display_list(&mut self, size: LogicalSize, _scale: f64) -> Option<DisplayList> {
+ // Whatever the engine last handed over is about to be on screen. That
+ // is what lets the next one be read: until a frame is drawn, reading
+ // another would be copying over a picture nobody saw.
+ #[cfg(feature = "wpe")]
+ self.host.frame_drawn();
self.win = (size.width, size.height);
let mut pc = PaintCtx::new();
let w = size.width;
diff --git a/src/wpe/host.rs b/src/wpe/host.rs
index dd0860b..986e180 100644
--- a/src/wpe/host.rs
+++ b/src/wpe/host.rs
@@ -107,12 +107,34 @@ unsafe fn connect_notify(wv: *mut WebKitWebView, signal: &str, state: &Rc<TabSta
);
}
-/// Frames handed over by `render_buffer`, drained by `pump`. A slot, not a
-/// queue: only the newest frame is worth uploading, and WPE will not produce
-/// another until we release the current one anyway.
+/// The frame handed over by `render_buffer`, drained by `pump`. A slot, not a
+/// queue: only the newest frame is ever shown, and the engine will not run far
+/// ahead of a browser that has not released the one it is holding.
+/// Counters behind `CCE_BROWSER_FRAME_DEBUG=1`: how many frames the engine
+/// finished against how many were actually read back. The gap between them is
+/// what pacing saves, and it is invisible from the outside — a browser that
+/// skips nine frames in ten looks exactly like one that copies all ten.
+#[derive(Default)]
+struct FrameCounts {
+ produced: u64,
+ read: u64,
+}
+
+fn frame_debug() -> bool {
+ static ON: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
+ *ON.get_or_init(|| std::env::var_os("CCE_BROWSER_FRAME_DEBUG").is_some())
+}
+
#[derive(Default)]
struct Pending {
- frame: Option<(Vec<u8>, u32, u32)>,
+ /// The newest finished buffer the engine has handed over, still unread.
+ /// Read and released at the next `pump`; superseded by a newer one, which
+ /// hands this one back **unread** — that skipped copy is the whole point
+ /// of holding it rather than copying in the callback.
+ held: Option<(*mut WPEView, *mut WPEBuffer)>,
+ counts: FrameCounts,
+ /// When the counters were last reported.
+ reported: Option<std::time::Instant>,
}
pub struct WebKitHost {
@@ -142,6 +164,14 @@ pub struct WebKitHost {
download_started: Rc<Cell<bool>>,
/// A page asked something and is blocked until we answer.
prompts: Rc<RefCell<Prompts>>,
+ /// A frame has been uploaded that nothing has drawn yet.
+ ///
+ /// The readback is paced by this: while it is set, a finished buffer is
+ /// left *held* instead of being copied, and the next engine frame hands it
+ /// back unread. An animating page in a window nobody is drawing — occluded,
+ /// on another desktop — therefore costs nothing, where before it copied
+ /// its full window size sixty times a second into a picture no one saw.
+ pending_draw: Cell<bool>,
/// The injected account watcher, kept so the setting can take it away
/// again. `None` when account autocomplete is off, which is also when no
/// page carries the script at all.
@@ -260,17 +290,20 @@ impl WebKitHost {
let prompts = Rc::new(RefCell::new(Prompts::default()));
let pending = Rc::new(std::cell::RefCell::new(Pending::default()));
let sink = pending.clone();
- FRAME_SINK = Some(Box::new(move |buffer: *mut WPEBuffer| {
+ FRAME_SINK = Some(Box::new(move |view: *mut WPEView, buffer: *mut WPEBuffer| {
let mut slot = sink.borrow_mut();
- // Replace, never accumulate: the newest frame wins. The
- // superseded frame's buffer is refilled rather than dropped —
- // when the engine outruns `pump`, which is exactly when frames
- // are being thrown away, allocating a new one each time would
- // be the most expensive possible way to discard work.
- let previous = slot.frame.take().map(|(px, ..)| px);
- if let Some(f) = read_shm(buffer, previous) {
- slot.frame = Some(f);
+ // Replace, never accumulate: the newest frame wins. The one it
+ // supersedes goes back to the engine **without being read** —
+ // several frames can be dispatched inside a single pump's
+ // drain, and only the last of them will ever be shown, so the
+ // rest are not worth 35 MB of copying each.
+ if let Some((old_view, old_buffer)) = slot.held.replace((view, buffer)) {
+ wpe_view_buffer_released(old_view, old_buffer);
}
+ if frame_debug() {
+ slot.counts.produced += 1;
+ }
+ true
}));
let toplevel = wpe_display_create_toplevel(display, 1);
@@ -299,6 +332,7 @@ impl WebKitHost {
clear_cookies,
session,
download_started,
+ pending_draw: Cell::new(false),
prompts,
last_pixel: None,
ucm: webkit_user_content_manager_new(),
@@ -558,6 +592,10 @@ impl WebKitHost {
}
let was_active = index == self.active;
let old_active = self.active;
+ // Anything still held belongs to a view that may be the one about to
+ // be destroyed; hand it back while it is still safe to. Losing that
+ // frame costs a repaint, which the tab change causes anyway.
+ self.release_held();
// Dropping the Tab unrefs the webview and frees its registry image.
drop(self.tabs.remove(index));
if self.tabs.is_empty() {
@@ -575,6 +613,13 @@ impl WebKitHost {
true
}
+ /// Give back an unread buffer, if one is being held.
+ fn release_held(&self) {
+ if let Some((view, buffer)) = self.pending.borrow_mut().held.take() {
+ unsafe { wpe_view_buffer_released(view, buffer) };
+ }
+ }
+
/// Make tab `index` visible and focused. Mirrors `ServoHost::activate`,
/// including the `usize::MAX` sentinel so the first call is not a no-op.
pub fn activate(&mut self, index: usize) {
@@ -669,8 +714,41 @@ impl WebKitHost {
if let Some(p) = &mut self.poll {
p.sync();
}
- let frame = self.pending.borrow_mut().frame.take();
+ // Nothing has drawn the last frame yet, so reading another would be
+ // copying over a picture that was never shown. Leave the buffer held:
+ // the engine's next frame supersedes it and hands it back unread.
+ if self.pending_draw.get() {
+ return (false, self.sync_page_state());
+ }
+ // One readback per pump, of the newest buffer only: everything the
+ // engine rendered in between was handed back unread.
+ let held = self.pending.borrow_mut().held.take();
let dirty = self.sync_page_state();
+ let Some((view, buffer)) = held else {
+ return (false, dirty);
+ };
+ let frame = unsafe {
+ let f = read_shm(buffer);
+ // The pixels are ours now; the memory can go back.
+ wpe_view_buffer_released(view, buffer);
+ f
+ };
+ if frame_debug() {
+ let mut p = self.pending.borrow_mut();
+ p.counts.read += 1;
+ let now = std::time::Instant::now();
+ let due = p.reported.is_none_or(|t| now.duration_since(t).as_secs_f32() >= 1.0);
+ if due {
+ p.reported = Some(now);
+ let (produced, read) = (p.counts.produced, p.counts.read);
+ p.counts = FrameCounts::default();
+ log::info!(
+ "frames: engine produced {produced}, read back {read} \
+ ({} handed back unread)",
+ produced.saturating_sub(read)
+ );
+ }
+ }
let Some((px, w, h)) = frame else {
return (false, dirty);
};
@@ -695,9 +773,16 @@ impl WebKitHost {
tab.image = Some((id, w, h));
}
}
+ self.pending_draw.set(true);
(true, true)
}
+ /// The chrome drew: whatever was uploaded is on screen, so the next
+ /// engine frame is worth reading. Called from `display_list`.
+ pub fn frame_drawn(&self) {
+ self.pending_draw.set(false);
+ }
+
/// Fold each tab's signal-written state into the fields the chrome reads.
///
/// Every tab, not just the active one — that is the whole point of moving
@@ -1164,11 +1249,11 @@ impl WebKitHost {
/// copying. What remains is one memcpy per row, and only when the stride
/// forces it — a tight stride is copied whole.
///
+/// Called from `pump`, never from the frame callback: a buffer superseded
+/// before the next pump is never read at all.
+///
/// The stride is not assumed to equal `width * 4`.
-unsafe fn read_shm(
- buffer: *mut WPEBuffer,
- reuse: Option<Vec<u8>>,
-) -> Option<(Vec<u8>, u32, u32)> {
+unsafe fn read_shm(buffer: *mut WPEBuffer) -> Option<(Vec<u8>, u32, u32)> {
if g_type_check_instance_is_a(buffer as *mut GTypeInstance, wpe_buffer_shm_get_type()) == 0 {
return None;
}
@@ -1188,14 +1273,7 @@ unsafe fn read_shm(
if (len as usize) < stride * (h as usize - 1) + row {
return None;
}
- let mut out = match reuse {
- Some(mut buf) if buf.len() == need => {
- // Every byte below is overwritten, so nothing has to be cleared.
- buf.truncate(need);
- buf
- }
- _ => cce_ui::vk::recycle_buffer(need),
- };
+ let mut out = cce_ui::vk::recycle_buffer(need);
if stride == row {
std::ptr::copy_nonoverlapping(src, out.as_mut_ptr(), need);
} else {
diff --git a/src/wpe/subclass.rs b/src/wpe/subclass.rs
index d9a994c..5af3f9f 100644
--- a/src/wpe/subclass.rs
+++ b/src/wpe/subclass.rs
@@ -85,7 +85,12 @@ pub(super) unsafe fn types() -> &'static Types {
/// Set by the host before it creates a webview; `render_buffer` hands frames
/// here. One host per process for now (see `WebKitHost::new`).
-pub(super) static mut FRAME_SINK: Option<Box<dyn FnMut(*mut WPEBuffer)>> = None;
+///
+/// Returns whether the sink is **keeping** the buffer. If it is, releasing it
+/// is the sink's job — it reads the pixels out at the next pump and hands the
+/// memory back then.
+pub(super) static mut FRAME_SINK: Option<Box<dyn FnMut(*mut WPEView, *mut WPEBuffer) -> bool>> =
+ None;
unsafe extern "C" fn view_render_buffer(
view: *mut WPEView,
@@ -94,16 +99,23 @@ unsafe extern "C" fn view_render_buffer(
_n_damage: u32,
_error: *mut *mut GError,
) -> gboolean {
+ // The two halves mean different things and are no longer said together.
+ // `rendered` means *displayed*: said at once, so the engine's own frame
+ // pacing never waits on our readback. `released` means *the memory is
+ // yours again*, and that has to wait until the pixels have been copied
+ // out of it — so the sink says it, at the pump that reads the buffer.
+ // (Saying neither is what stalls the engine after exactly one frame.)
+ //
+ // Holding the buffer until then is also the backpressure: the engine
+ // cannot run arbitrarily far ahead of a browser that is not keeping up,
+ // and a frame superseded before anyone read it is handed back unread
+ // rather than copied.
+ wpe_view_buffer_rendered(view, buffer);
#[allow(static_mut_refs)]
- if let Some(sink) = FRAME_SINK.as_mut() {
- sink(buffer);
+ let held = FRAME_SINK.as_mut().is_some_and(|sink| sink(view, buffer));
+ if !held {
+ wpe_view_buffer_released(view, buffer);
}
- // BOTH halves. `rendered` means displayed, `released` means the memory is
- // yours again; with only the first the engine produces exactly one frame
- // and then stalls forever. This is also the backpressure that makes an
- // unbounded upload queue impossible here.
- wpe_view_buffer_rendered(view, buffer);
- wpe_view_buffer_released(view, buffer);
1
}