fix: snapshot activeAnimations in update() to prevent stalls from mid-iteration splice#839
Merged
Merged
Conversation
…-iteration splice AnimationManager.update() iterated activeAnimations with a cached length, but animations that finish during iteration call unregisterAnimation() which splices the array. With the cached length, subsequent indices read undefined or skip animations, causing animation-heavy scenarios like confetti to stall. Fix: snapshot the array with slice() before iterating, matching the same pattern used in EventEmitter.emit().
Scale POOL_SIZE and BURST_COUNT by perfMultiplier so the test can be stressed further on high-end hardware via ?multiplier=N in the URL. Default (multiplier=1) is unchanged: 160 pool + 30 burst particles.
Three fixes bundled together: 1. Pool corruption (the activeAnimations infinite growth bug) CoreAnimationController.onFinished/stop/onDestroy now capture local refs to animation and manager before emitting 'stopped'. Previously, the user's stopped callback could synchronously call createAnimation() which pops the just-finished animation from the pool and re-inits it, then releaseToPool() would immediately clear its listeners, corrupting the newly-started animation. Result was activeAnimations growing unboundedly (starts at 14fps, bottoms out to 1fps). 2. EventEmitter.emit() - backwards for loop replaces slice() snapshot listeners.slice() allocated a new array on every emit() call. With 380 concurrent animations each emitting 'tick' every frame, this created ~22,800 short-lived arrays per second. A backwards for loop is equally safe against once()/off() splice() mutations (removals shift elements at higher indices, already visited) with zero allocations. 3. AnimationManager.update() + unregisterAnimation() Backwards for loop replaces the animations.slice() snapshot (one 380-element copy per frame). Swap-remove restored in unregisterAnimation() for O(1) removal. A swap-remove at index i only affects elements at index >= i, which a backwards loop has already visited, so no element is ever skipped.
Six targeted fixes to eliminate the allocation pressure identified in
heap profiling (85k arrays, 85k Arrays, 104k numbers created/deleted):
1. CoreAnimation: PropGroup arrays reused across pool recycles
Instead of allocating 4 new arrays + 1 PropGroup object on every
animation recycle, we now keep persistent PropGroup instances on the
CoreAnimation and clear/refill them via a length counter. Eliminates
~5 object allocations per animation lifecycle.
2. CoreAnimation: emit() called without data payload
All emit('finished',{}), emit('animating',{}), emit('destroyed',{})
calls now pass no data arg (undefined). The {} argument was a fresh
object allocation on every state transition that listeners never used.
3. EventEmitter: removeAllListeners() clears in place
Instead of this.eventListeners = {} (new object every call),
we now delete keys from the existing object. Eliminates one {} per
removeAllListeners() call, which fires twice per animation lifecycle
(in init() and releaseToPool()).
4. utils.ts: Fix broken Newton-Raphson guard
cbxd > 1e-10 && cbxd < 1e-10 is an impossible condition (always false)
causing the near-zero derivative fallback to never trigger. Fixed to
cbxd > -1e-10 && cbxd < 1e-10. Previously the solver always ran all
20 Newton iterations before the binary search, wasting ~190k FP ops/frame.
5. CoreAnimation: linear easing takes O(1) fast path
easing && easing !== 'linear' guard prevents 'linear' (a truthy string)
from calling the timingFunction closure when the inlined calculation
startValue + (endValue - startValue) * progress is identical and cheaper.
6. Stage: pre-allocated frameTick payload
Reuse a single { time, delta } object instead of allocating a new one
every frame for the 'frameTick' event.
…lag, no Math.trunc
Implements all remaining bottlenecks identified in heap profiling:
1. EventEmitter: add clearListeners(events) method
Zeros known listener arrays in-place (arr.length=0) without deleting
keys, keeping V8 in fast-properties mode and preserving the arrays
for reuse. Prevents hidden-class demotion caused by 'delete'.
2. CoreAnimation + CoreAnimationController: pre-allocate listener arrays
Constructor now pre-allocates [] for each known event name so that
registerAnimation()'s on() calls never need to allocate new arrays.
releaseToPool() now calls clearListeners() instead of removeAllListeners()
to preserve those arrays across pool recycles.
Eliminates 4 array allocations per animation start.
3. CoreAnimation: isLinear boolean flag
Replaces the per-frame 'easing !== linear' string comparison in
updateValue() with a boolean flag set once in init(). Also removes
the now-unused easing parameter from updateValue/updateValues since
all branching is via this.isLinear.
4. CoreAnimationController: pre-allocated tickPayload
Reuses { progress: 0 } object across frames. Also switches onTick
from forEach to a reverse for loop.
5. CoreAnimation.update(): extract propValuesMap to locals
Avoids repeated bracket-string property lookups in the hot path.
Also removes 'easing' from the update() destructure since it is
no longer needed.
6. utils.ts mergeColorProgress: remove Math.trunc()
>>> and & 0xff already produce unsigned integers. Math.trunc()
was a no-op wrapping 8 redundant function calls per color blend.
7. confetti.ts: reuse animations array on relaunch
this.animations.length=0 + push() instead of allocating new []
on every particle relaunch.
…checks
1. isLinear -> hasEasing with explicit === true / === false checks
Renames the cached easing flag to 'hasEasing' (positive branch, no
negation) and uses strict boolean comparison on all checks per the
requirement of direct comparisons, no truthy/falsy.
2. invDuration: replace per-frame division with multiplication
Store 1/duration in init() so update() does:
progress += dt * this.invDuration
instead of:
progress += dt / duration
Float division is ~5-20x slower than multiplication on modern CPUs.
3. Cache progress in local variable in update()
Read this.progress once into a local 'progress' at the start of
update(), do all arithmetic on the local, write back once at the
end. Avoids repeated this.progress reads which cause V8 to unbox
the HeapNumber on every property access -- the source of the
-0.x float allocations visible in the heap profiler.
4. Pass progress as parameter to updateValue() / updateValues()
The float stays on the stack (unboxed register) as it crosses the
function boundary, rather than being re-read from the object heap.
5. Inline applyEasing
Removes the private applyEasing() method. The one-liner
timingFunction(p) * (e - s) + s is inlined directly in updateValue(),
eliminating a function call per non-linear property per frame.
6. Explicit === comparisons throughout update() and updateValue()
loop === true, stopMethod !== false, hasEasing === true,
isColor === true -- all direct type-safe comparisons.
confetti2 differs from confetti in one key way: nodes are ephemeral. Each particle is a brand-new node that gets created, animated, and destroyed on completion -- nothing is reused between particles. This stresses the full animation lifecycle continuously: - Node create/destroy every cycle (no node pooling) - Animation controller + animation init on every spawn - 3 concurrent animations per particle (Y/X/rotation) - Constant pool churn even at steady state A new wave of WAVE_SIZE (50 * perfMultiplier) particles fires every 500ms, overlapping with the previous wave still in flight. Use ?multiplier=N to scale the load. Use this alongside confetti (which recycles nodes) to separately benchmark: - confetti: animation pool recycling throughput - confetti2: full spawn/destroy churn throughput
…stent nodes Nodes are created once and kept alive. Each cycle creates 3 brand-new animation controllers (Y/X/rotation), starts them, and immediately re-creates fresh ones on completion. This isolates the createAnimation() churn cost from node overhead. Key differences vs confetti: - Nodes: persistent (created once, never destroyed) - Animations: 3 per node (Y linear, X cubic-bezier, rotation ease-out) - Controllers: fresh every cycle via node.animate() -- stresses pool - Relaunch: immediate on Y stop, no rest period Scale: 50 * perfMultiplier nodes via ?multiplier=N
…steners Two fixes targeting the confetti2 FPS regression (18 vs 23 FPS vs 3.1.1): 1. O(1) unregisterAnimation via activeIndex Add activeIndex: number to CoreAnimation, kept in sync on every registerAnimation() and swap-remove. unregisterAnimation() now reads animation.activeIndex directly instead of indexOf() scan. Restores O(1) removal matching v3.1.1's Map.delete(). registerAnimation(): sets animation.activeIndex = array.length before push unregisterAnimation(): uses activeIndex directly, updates swapped element's index, sets activeIndex = -1 on removal -- O(1) total update() checks anim.activeIndex >= 0 before calling update(dt), preventing double-processing when a sibling stop() during a completion callback swap-removes an already-visited element into the current slot. 2. Remove double clearListeners from init() releaseToPool() already calls clearListeners() on both animation and controller before returning them to the pool. init() was redundantly calling clearListeners() again on already-empty arrays. Removed from CoreAnimation.init() and CoreAnimationController.init(). Contract: objects arrive at init() already cleared by releaseToPool().
…reset, emit guard Four changes to reduce synchronous work inside the rAF handler during animation completion, improving stability (P01/P05 FPS) under continuous animation churn (confetti2 pattern): 1. EventEmitter.emit(): early exit on listeners.length === 0 Pre-allocated empty arrays (stopped/animating/tick/destroyed) now short-circuit immediately without entering the loop -- single integer comparison instead of a full loop iteration with no-op. 2. EventEmitter.clearListeners(): arr.length > 0 guard Only writes arr.length = 0 when there is actually something to clear. In steady-state (after unregisterAnimation() empties arrays via off()), this is 4-6 integer reads and zero writes -- avoids write barriers on old-gen pooled objects. 3. CoreAnimation.reset(): remove update(0), inline restoreValues directly update(0) ran the full update pipeline (dirty marking, event emissions, updateValues) just to write start values back to node properties. reset() now writes start values directly via restoreValues(). restore() simplified to just call reset(). Identical visible behaviour. 4. Swap: clearListeners moved from releaseToPool() to init() releaseToPool() is now just 2 array pushes -- the minimum possible work in the hot rAF completion path. clearListeners() runs lazily in init() on the next reuse, where the arr.length > 0 guard makes it nearly free (arrays already empty after unregisterAnimation).
jfboeve
approved these changes
Jul 13, 2026
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
2 participants
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
Fixes confetti (and other animation-heavy scenarios) stalling after the animation pool and flat-array changes were merged.
Root Cause
AnimationManager.update()iteratedactiveAnimationswith a cached length:When an animation finishes during iteration,
onFinished()callsunregisterAnimation()which doessplice(index, 1)on the live array. With the cachedlen, subsequent indices either readundefined(crash) or skip animations that shifted down, causing them to never advance and eventually stall.Fix
Snapshot the array with
slice()before iterating, matching the same pattern already used inEventEmitter.emit():Testing