gpui: Retain clean entity views - #30
Open
huacnlee wants to merge 10 commits into
Open
Conversation
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EntityandAnyViewsubtrees by default.retained(id, version, build)boundary for unchanged subtrees inside an entity that must rerender, such as static input contents beside a blinking cursor.ListState,ScrollHandle, andUniformListScrollHandleinvalidate their owning view when element-local state changes outside the Entity notification graph.Root cause
A GPUI notification reruns the changed Entity's
Renderimplementation. Before this change, ordinaryViewElements did not usedirty_viewsto prune clean sibling Entities, so layout preparation, prepaint, text work, and paint were repeated across unrelated visible subtrees even when only a cursor or scroll offset changed.The production Editor phase benchmark confirms that Taffy layout is not the main cost for these small updates. A no-op Editor notification spends about
457.5 µstotal, including about248 µsin prepaint and133 µsin paint, while layout computation is below1 µs.A benchmark that toggles the real
BlinkManagervisibility state confirms that a cursor blink and a no-op Editor notification take the same full redraw path. In an adjacent run, the cursor blink was2.5%faster than the no-op notification, with no statistically significant difference. Their phase distributions were also equivalent: prepaint dominated, paint was second, and request plus computed layout remained below0.005 ms. These thermally adjacent measurements establish path equivalence; their absolute values are not mixed with the cold before/after table below.Incremental rendering model
Ordinary
Entity<T>andAnyViewelements now retain their previous subtree automatically when the backing Entity and every Entity accessed while rendering it remain clean. Reuse is limited to roots with definite width and height; intrinsic-sized roots continue through normal rendering.Once a clean View is known to have an intrinsic root, later parent redraws use the ordinary render path directly instead of repeating retained dependency detection. A notification on that View rechecks eligibility, so a root that changes from intrinsic to definite-sized can still begin retaining.
The cache is also invalidated by changes to bounds, content mask, inherited text style, rem size, scale factor, explicit window refresh, and Inspector picking.
Some GPUI elements keep mutable state outside an Entity.
ListStateand scroll handles now capture a weak, view-scoped invalidator during prepaint. Their public mutation methods mark that View dirty without emitting an Entity observer notification. Mutations made while the current frame is already being built do not enqueue another invalidation.The versioned retained element remains available for finer boundaries inside one dirty Entity:
Its stable ID identifies the boundary and its version must cover every captured value that affects the subtree.
Benchmarks
Apple Silicon Mac, production Editor with 1,000 lines and an unchanged 1,000-row sibling Entity. Criterion used 100 samples for the final default-path runs.
Criterion's stored-baseline comparisons reported
-69.8%for the no-op update and-59.7%for scrolling.The synthetic same-Entity benchmark isolates the finer-grained case:
Correctness boundaries
Entity<T>andAnyViewconversions opt into automatic retention. A customViewthat combines parent props with an Entity identity remains uncached unless it explicitly chooses otherwise.An earlier attempt to retain all clean Entities failed five list remeasurement/follow-tail tests because element-local state could change without
Context::notify. The view invalidator closes that gap; the complete GPUI suite now passes with automatic retention enabled.Test Plan
cargo test -p gpui --features test-support --lib(223 passed)cargo test -p workspace --lib(237 passed)./script/clippy -p gpui --features test-support./script/clippy -p workspacecargo bench -p gpui --bench small_updates --features bench --no-runcargo bench -p benchmarks --bench editor_render --no-runcargo bench -p benchmarks --bench editor_render -- editor_noop_notify_with_static_siblingcargo bench -p benchmarks --bench editor_render -- editor_cursor_blinkcargo bench -p benchmarks --bench editor_render -- editor_scroll_with_static_siblingSuggested .rules additions
When adding mutable state handles to GPUI elements, ensure mutations made outside rendering can invalidate the Entity View that last rendered the handle. Entity cleanliness alone is not sufficient if an element owns independently mutable state.
Release Notes: