Skip to content

Commit a8e2cec

Browse files
committed
perf(destroyable): replace splice with swap-and-pop in remove()
`remove()` in `@glimmer/destroyable/index.ts:58` is invoked every time a destroyable is unassociated from its parent (happens during the destroy phase, from `removeChildFromParent`). Its previous form: let index = collection.indexOf(item); collection.splice(index, 1); does an O(n) shift of all elements after `index`. For a parent with thousands of children being cleared, the total cost is O(n²). Swap-with-last-then-pop is O(1) for the splice piece (the indexOf lookup above it is unchanged): let index = collection.indexOf(item); let lastIndex = collection.length - 1; if (index !== lastIndex) { collection[index] = collection[lastIndex] as T; } collection.pop(); Order is not observable. The only consumer of the collection is `iterate()` (uses `.forEach` for destructors/parents/children); none of the callers — destructor firing, parent/child propagation — assume sibling order. Parent-side removal is also batched via `scheduleDestroyed`, so a child is never removed from the collection while the parent is iterating it. Measured with tracerbench (20-fidelity compare vs origin/main): | phase | Δ ms | Δ % | |----------------------|------------:|---------:| | clearManyItems1End | **-43 ms** | **-21.3 %** | | clearManyItems2End | **-40 ms** | **-39.5 %** | | render1000Items1End | -2 ms | -2.9 % | | (all other 17 phases)| no diff | | 90% CIs for the two clear-5000 phases: [-46, -40] ms and [-46, -36] ms respectively — well outside the regression/noise threshold tracerbench uses. No regressions on any measured phase.
1 parent 0d73c63 commit a8e2cec

1 file changed

Lines changed: 5 additions & 1 deletion

File tree

packages/@glimmer/destroyable/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ function remove<T extends object>(collection: OneOrMany<T>, item: T, message: st
6767

6868
if (isBrandedArray(collection) && collection.length > 1) {
6969
let index = collection.indexOf(item);
70-
collection.splice(index, 1);
70+
let lastIndex = collection.length - 1;
71+
if (index !== lastIndex) {
72+
collection[index] = collection[lastIndex] as T;
73+
}
74+
collection.pop();
7175
return collection;
7276
} else {
7377
return null;

0 commit comments

Comments
 (0)