perf(engine-core): avoid Array#splice in mutation tracking

[nolanlawson]

Details

Instead of doing an expensive Array#splice in this perf-sensitive mutation tracking code, we can

1. swap the target item with the last item
2. call Array#pop instead

I couldn't find an existing benchmark to repro the improvement in Chrome, so I wrote a new one. This new test builds a tree structure with recursive components. The reason it repros the improvement is that it has lots of component instances that are tracking mutations on a single getter (which is probably something that happens in the real world fairly frequently).

The improvement is 20-24%:

Here it is in a manual test to show the improvement to the reset() function:

FWIW I got this idea from Svelte here.

(I assume that the reason the splice is slow is because it has to create an additional array under the hood, plus it has to shift a bunch of array items around.)

Does this pull request introduce a breaking change?

• 😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

• 🤞 No, it does not introduce an observable change.

This is not an observable change because all the pending rehydration callbacks are queued up in a microtask, and then we sort the queue before running them:

lwc/packages/@lwc/engine-core/src/framework/vm.ts

Line 667 in 6c46adb

const vms = rehydrateQueue.sort((a: VM, b: VM): number => a.idx - b.idx);

GUS work item

[nolanlawson]

Added this benchmark for the heck of it. There is no improvement from this PR, since this PR targets reset() which happens during component removal.

[wjhsf]

Is there a performance difference between set.pop() and set.length = setLength - 1? We use the former here, but the latter in the if block.

[nolanlawson]

That's a good catch. We can actually refactor the code to simplify it, if we're always just poping or setting set.length = length - 1 in every case.

76f07c4

I did a quick perf test, and there's basically no difference between set.length = length - 1:

... and pop():

Although maybe pop() is slightly faster? Either way I would prefer pop() since it's more idiomatic.

I'm also planning to run this through our full benchmark suite tonight, just in case there is some weird edge case I missed.

[nolanlawson]

Huh, so it looks like pop() is faster than setting length. In a minimal comparison, the benchmark reports that pop() is 1-4% faster:

[wjhsf]

Making the file 55 characters longer to save 2 characters in the filename! 🚀

[nolanlawson]

OK, running the full benchmark suite of this PR against master, it looks like it's basically pure improvement. (There's one 1ms regression that I'm not too concerned about.)