From 91df575cf81d0ccc479891e3f6189fd6550d3811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 16 May 2026 04:23:49 -0400 Subject: [PATCH] perf(app): batch scroll surface flushes --- crates/ely_app/src/shell/web_surface.rs | 43 ++++++++++--------- .../src/shell/web_surface_controller.rs | 9 +--- .../src/shell/web_surface_runtime_tests.rs | 12 +++--- crates/ely_app/src/shell/web_surface_state.rs | 25 +++++++++++ 4 files changed, 55 insertions(+), 34 deletions(-) diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index d8a08bd..cbab5ab 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -48,20 +48,19 @@ impl WebSurfaceStore { tab: &BrowserTab, profile_data_mode: ProfileDataMode, permissions: &[WebSurfaceSitePermission], - ) -> WebSurfaceEnsureOutcome { + ) -> bool { if !is_external_web_url(tab.url().as_str()) { - return WebSurfaceEnsureOutcome::default(); + return false; } - let requested_url = tab.url().as_str().to_string(); let Some(size) = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size) else { - return WebSurfaceEnsureOutcome::default(); + return false; }; let ensure_key = WebSurfaceEnsureKey::new(requested_url.clone(), size, tab.zoom_percent(), permissions); if self.surfaces.get(tab.id()).is_some_and(|surface| !surface.should_ensure(&ensure_key)) { - return WebSurfaceEnsureOutcome::default(); + return false; } let input = self.take_pending_input(tab.id(), requested_url.as_str()); let previous_frame = @@ -75,14 +74,14 @@ impl WebSurfaceStore { requested_url: result.requested_url, previous_frame, }); - return WebSurfaceEnsureOutcome { changed: true, url_change: None }; + return true; } - WebSurfaceEnsureOutcome::default() + false } Err(message) => { self.surface_mut(tab.id()).mark_ensured(ensure_key); self.surface_mut(tab.id()).state = Some(WebSurfaceState::Failed { message }); - WebSurfaceEnsureOutcome { changed: true, url_change: None } + true } } } @@ -162,7 +161,7 @@ impl WebSurfaceStore { let stale_tab_ids = self .surfaces .keys() - .filter(|tab_id| !open_tab_ids.iter().any(|open_tab_id| open_tab_id == *tab_id)) + .filter(|tab_id| !open_tab_ids.contains(*tab_id)) .cloned() .collect::>(); for tab_id in stale_tab_ids { @@ -170,10 +169,6 @@ impl WebSurfaceStore { } } - pub(super) fn close_surface(&mut self, tab_id: &TabId) { - self.close_surface_for_tab(tab_id); - } - pub(super) fn record_scroll_delta( &mut self, tab_id: &TabId, @@ -196,6 +191,7 @@ impl WebSurfaceStore { }; let surface = self.surface_mut(tab_id); + let flush_throttled = surface.input_flush_is_throttled(Instant::now()); let scroll = surface .scroll_offset .get_or_insert_with(|| WebSurfaceScrollState::new(requested_url.to_string())); @@ -211,7 +207,11 @@ impl WebSurfaceStore { surface.pending_scroll_point = Some(point); surface.mark_pending_input_started(); surface.click_point = None; - WebSurfaceInputOutcome::Applied + if flush_throttled { + WebSurfaceInputOutcome::Buffered + } else { + WebSurfaceInputOutcome::Applied + } } pub(super) fn record_viewport_size( @@ -381,6 +381,13 @@ impl WebSurfaceStore { .map(|state| state.text); let hover_point = surface.hover_point.take(); let enqueued_at = surface.pending_input_started_at.take(); + if scroll_delta.is_some() + || click_point.is_some() + || hover_point.is_some() + || typed_text.is_some() + { + surface.mark_input_flushed(Instant::now()); + } WebSurfacePendingInput { enqueued_at, @@ -453,7 +460,7 @@ impl WebSurfaceStore { self.surfaces.entry(tab_id.clone()).or_insert_with(PerTabSurface::new) } - fn close_surface_for_tab(&mut self, tab_id: &TabId) { + pub(super) fn close_surface(&mut self, tab_id: &TabId) { self.runtime.close_tab(tab_id); self.surfaces.remove(tab_id); if self.keyboard_focus.as_ref().is_some_and(|focus| focus.tab_id == *tab_id) { @@ -476,12 +483,6 @@ pub(super) fn is_external_web_url(url: &str) -> bool { url.starts_with("https://") || url.starts_with("http://") } -#[derive(Default)] -pub(super) struct WebSurfaceEnsureOutcome { - pub(super) changed: bool, - pub(super) url_change: Option, -} - #[derive(Default)] pub(super) struct WebSurfaceTickResult { pub(super) changed: bool, diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 1cfd385..2058427 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -182,20 +182,15 @@ impl ElyShell { impl ElyShell { fn ensure_visible_web_surfaces(&mut self, visible_tabs: Vec) -> bool { - let mut url_changed = false; let mut changed = false; for visible in visible_tabs { - let outcome = self.web_surfaces.ensure_surface( + changed |= self.web_surfaces.ensure_surface( &visible.tab, visible.profile_data_mode, &visible.permissions, ); - changed |= outcome.changed; - if let Some(url_change) = outcome.url_change { - url_changed |= self.apply_web_surface_url_change(url_change); - } } - changed || url_changed + changed } fn apply_web_surface_url_change(&mut self, change: WebSurfaceUrlChange) -> bool { diff --git a/crates/ely_app/src/shell/web_surface_runtime_tests.rs b/crates/ely_app/src/shell/web_surface_runtime_tests.rs index 2abd79f..bf00811 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_tests.rs @@ -103,11 +103,11 @@ fn unchanged_surface_without_input_skips_runtime_ensure() -> Result<(), String> store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), WebSurfaceInputOutcome::Applied, ); - assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); store.flush_runtime_for_test(); assert_eq!(IDLE_SKIP_ENSURE_COUNT.load(Ordering::SeqCst), 1); - assert!(!store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + assert!(!store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); store.flush_runtime_for_test(); assert_eq!(IDLE_SKIP_ENSURE_COUNT.load(Ordering::SeqCst), 1); Ok(()) @@ -126,7 +126,7 @@ fn store_tick_delay_tracks_runtime_cadence() -> Result<(), String> { store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), WebSurfaceInputOutcome::Applied, ); - assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); assert_eq!(store.next_tick_delay(&visible), Duration::ZERO); let _ = store.tick(&visible); @@ -188,13 +188,13 @@ fn failed_surface_ensure_waits_for_a_new_key_before_retrying() -> Result<(), Str store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), WebSurfaceInputOutcome::Applied, ); - assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); store.flush_runtime_for_test(); let tick = store.tick(&[tab.id().clone()]); assert!(tick.changed, "the failing client must surface a state change via tick"); assert_eq!(FAILING_ENSURE_COUNT.load(Ordering::SeqCst), 1); - assert!(!store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + assert!(!store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); store.flush_runtime_for_test(); let _ = store.tick(&[tab.id().clone()]); assert_eq!(FAILING_ENSURE_COUNT.load(Ordering::SeqCst), 1); @@ -203,7 +203,7 @@ fn failed_surface_ensure_waits_for_a_new_key_before_retrying() -> Result<(), Str store.record_viewport_size(tab.id(), resized_viewport_bounds(), 1.0), WebSurfaceInputOutcome::Applied, ); - assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); store.flush_runtime_for_test(); let _ = store.tick(&[tab.id().clone()]); assert_eq!(FAILING_ENSURE_COUNT.load(Ordering::SeqCst), 2); diff --git a/crates/ely_app/src/shell/web_surface_state.rs b/crates/ely_app/src/shell/web_surface_state.rs index e915013..5d0e870 100644 --- a/crates/ely_app/src/shell/web_surface_state.rs +++ b/crates/ely_app/src/shell/web_surface_state.rs @@ -4,6 +4,7 @@ use ely_domain::TabId; use gpui::{Bounds, Pixels}; use super::{ + web_surface_cadence::ACTIVE_POLL_INTERVAL, web_surface_frame::WebSurfaceFrame, web_surface_geometry::{ WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize, @@ -70,6 +71,8 @@ pub(super) struct WebSurfacePendingInput { pub(super) enum WebSurfaceInputOutcome { /// State changed and the renderer should re-notify. Applied, + /// State changed and the active cadence timer will flush it. + Buffered, /// Same value as currently recorded — nothing to flush downstream. NoChange, /// Geometry constructor rejected the input (zero/NaN/negative @@ -124,6 +127,7 @@ pub(super) struct PerTabSurface { pub(super) scroll_offset: Option, pub(super) typed_text: Option, pub(super) state: Option, + last_input_flushed_at: Option, } impl PerTabSurface { @@ -141,6 +145,7 @@ impl PerTabSurface { scroll_offset: None, typed_text: None, state: None, + last_input_flushed_at: None, } } @@ -157,6 +162,15 @@ impl PerTabSurface { self.last_hover_enqueued_at = Some(now); } + pub(super) fn input_flush_is_throttled(&self, now: Instant) -> bool { + self.last_input_flushed_at + .is_some_and(|last| now.duration_since(last) < ACTIVE_POLL_INTERVAL) + } + + pub(super) fn mark_input_flushed(&mut self, now: Instant) { + self.last_input_flushed_at = Some(now); + } + pub(super) fn should_ensure(&self, key: &WebSurfaceEnsureKey) -> bool { self.last_ensure_key.as_ref() != Some(key) || self.has_pending_input() } @@ -242,6 +256,17 @@ mod tests { assert!(surface.should_ensure(&new_key)); } + #[test] + fn recent_input_flush_throttles_immediate_flush() { + let start = Instant::now(); + let mut surface = PerTabSurface::new(); + + surface.mark_input_flushed(start); + + assert!(surface.input_flush_is_throttled(start + Duration::from_millis(7))); + assert!(!surface.input_flush_is_throttled(start + Duration::from_millis(8))); + } + fn ensure_key(url: &str, width: u32, height: u32) -> WebSurfaceEnsureKey { WebSurfaceEnsureKey::new( url.to_string(),