Raise retained widget layout budget 1024 -> 2048 nodes per view - #409
Open
slicenferqin wants to merge 1 commit into
Open
Raise retained widget layout budget 1024 -> 2048 nodes per view#409slicenferqin wants to merge 1 commit into
slicenferqin wants to merge 1 commit into
Conversation
A production three-pane app (session rail ~50 rows, streaming detail timeline, model-picker dialog listing ~60 rows) measures 1036 retained widget nodes on its dialog frame — 12 over the 1024 cap. The WidgetLayoutListFull adoption failure discards the whole frame, so opening the dialog leaves every control visibly dead, and a view that sits over budget wedges interactions on every invalidated frame. Doubling the budget covers the measured view with ~2x headroom. The automation snapshot cap moves in lockstep (the existing lockstep test enforces it). 4096 was ruled out: session record/replay tests inline record+replay harnesses into one test frame (~11 MiB at 1024 on macOS), and 4096 pushes those frames past the main-thread stack limit (53 crashes); 2048 keeps zig build test green. Fixes vercel-labs#408
|
@slicenferqin is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
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
1 participant
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.
Fixes #408.
What
Doubles the retained per-view widget budgets:
canvas_limits.max_canvas_widget_nodes_per_view1024 → 2048canvas_limits.max_canvas_widget_semantics_per_view1024 → 2048automation.snapshot.max_widgets_per_viewin lockstep (the existing lockstep test incanvas_widget_layout_tests.zigenforces the mirror)Why
The 1024 cap was sized against "the measured worst realistic three-pane view is ~500 nodes". Real list-dense views blow past that: our production app (session rail enumerating ~50 rows, streaming detail timeline, model-picker dialog listing ~60 models with provider-group headers) measures 1036/1024 nodes on its dialog frame — 12 nodes over.
The failure mode is much worse than "fail loudly":
adoptWidgetLayout→WidgetLayoutListFulldiscards the whole rebuild, so the interaction that pushed the view over budget (opening a dialog) appears to do nothing — controls look dead — and any view that sits over budget drops every subsequent frame, wedging its interactions entirely.2048 covers the measured view with ~2x headroom. These are validation bounds over fixed-capacity address space (pages touched only as used), so the raise costs address space, not committed memory.
Why not 4096
Tried it first. The session record/replay tests (
session_tests,media_surface_tests,effects_video_tests, …) inline record+replay harnesses and UiApp constructions into a single test function; on macOS those frames already run ~11 MiB of stack at 1024, and 4096 pushes them to ~17 MiB — past the main-thread stack limit: 53 tests crash with SIGSEGV at frame entry. At 2048zig build testis green.Longer term, apps retaining unbounded collections will outgrow any fixed cap; windowed retention for list widgets would remove the cliff. (Happy to open a separate issue for that.)
Test
zig build test(zig 0.16.0, macOS arm64): 3366/3381 pass, 14 skipped, 1 failed — the one failure (ui_app_tests.test.a failed build keeps the still-matching pair live) reproduces identically on unmodifiedmainwhen the shard runs in isolation (markup-build cache-state flake), so it is not caused by this change.