perf(app): batch scroll surface flushes
This commit is contained in:
@@ -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::<Vec<_>>();
|
||||
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<WebSurfaceUrlChange>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub(super) struct WebSurfaceTickResult {
|
||||
pub(super) changed: bool,
|
||||
|
||||
@@ -182,20 +182,15 @@ impl ElyShell {
|
||||
|
||||
impl ElyShell {
|
||||
fn ensure_visible_web_surfaces(&mut self, visible_tabs: Vec<VisibleWebSurfaceTab>) -> 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 {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<WebSurfaceScrollState>,
|
||||
pub(super) typed_text: Option<WebSurfaceTextInputState>,
|
||||
pub(super) state: Option<WebSurfaceState>,
|
||||
last_input_flushed_at: Option<Instant>,
|
||||
}
|
||||
|
||||
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(),
|
||||
|
||||
Reference in New Issue
Block a user