Read List's item-animation gating state once per pass instead of inside every item - #506
Read List's item-animation gating state once per pass instead of inside every item#506Aecasorg wants to merge 1 commit into
Conversation
…ry item shouldAnimateItems() was invoked inside each of the four item-production lambdas, so every visible item's own composition scope subscribed to forceUnanimatedItems and _searchableState.isSearching. A single flip of either — and forceUnanimatedItems flips on a 300ms timer after any unanimated recomposition — therefore invalidated every visible row individually and re-ran its content. That is wasted work in a transpiled app and much more costly in a Fuse app, where re-running a row rebuilds its view tree across the bridge. The predicate's value is identical for all items within a pass, so it is now computed once in the LazyColumn content scope and captured. No behavior change. Part of skiptools#486. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit b54538b)
|
Closing this — the change does not compile, and I should have caught that before opening the PR. Apologies for the noise. Hoisting the predicate out of the item lambdas moves It passed Worth recording why I don't think there is a simple corrected version: the only genuinely |
Small self-contained fix from the investigation written up in #486.
Problem
RenderListbuilds the item-animation predicate as a@Composableclosure and calls it from inside each of the four item-production lambdas:Because the reads happen inside the item lambda, every visible item's own composition scope subscribes to
forceUnanimatedItemsand to_searchableState.isSearching. A single flip of either then invalidates every visible row individually and re-runs its content.forceUnanimatedItemsis not rare: it is settrueon any unanimated recomposition and flipped back 300 ms later by theLaunchedEffectat L267-276.The predicate's value is identical for every item within a pass, so nothing is gained by evaluating it per item.
Change
Compute it once in the
LazyColumncontent scope (next to the existingmoveTriggerread) and capture theBoolin the item lambdas. Strictly fewer subscribing scopes; same value everywhere it is used; no behavior change.Verification
swift buildclean (transpile + macOS compile) on top ofmain.The user-visible cost of the current behavior is much larger in a Fuse app than a transpiled one — re-running a row there rebuilds its view tree across the bridge (two JNI hops plus a fresh peer allocation per bridged view per level), so a flip that re-runs ten visible rows is meaningfully expensive. Details in #486.
Not included
The
LaunchedEffect(System.currentTimeMillis())at L267-276 restarts on everyRenderListpass because its key is wall-clock time, and its delayed write flipsforceUnanimatedItemsback — which recomposes the list, which restarts the effect. That looks like a self-sustaining ~300 ms cycle for any list that is not animating, and this PR does not address it: every data-version signal we considered as a replacement key (item count, first/last identifier) changes animation behavior for same-count data replacements, which is a visible regression we did not want to introduce blind. Happy to take direction on the intended semantics there and follow up — it seems the more valuable of the two fixes.