vue icon indicating copy to clipboard operation
vue copied to clipboard

Destroy performance in Chromium browsers on macOS (w. potential solution)

Open kevinlee11 opened this issue 1 year ago • 6 comments

Version

2.7.8

Reproduction link

codesandbox.io/s/spqkdn

Steps to reproduce

We’ve recently noticed a performance problem in our Vue 2 application when destroying 100+ item components in our grid, it can take a significant amount of time (30+ seconds). Each of our item components contains a lot of child components that all use a number of computed properties which is the root cause of this issue, as looking into how the destroy process works, it needs to clean all of them up. After doing some further digging we discovered this issue is ONLY occurring on Chromium browsers on macOS (tested on Chrome and Edge). Other browsers do not see this destroy delay.

Further performance digging has found the root cause to be the Array.prototype.splice() in the remove function used by Vue in the destroy process.

image

In the name of looking for an easy win, simply switching the logic in the remove function to use Array.protoype.pop() instead fixes the issue.

image

Of course, this approach would only work if the order of the arrays passed into the remove function did NOT matter. Looking at the source code I can see there’s a number of files that make use of this remove function in the util file, and I don’t know the codebase well enough to understand the ramifications of this particular approach.

Could be we might need to create an alternative remove function that can be called by callers who don’t care about the order of their arrays (assuming some do care about the order)?

Hoping some people can chime in on this, thanks! 🙂

Note the attached minimal reproduction is a very simplified version of the issue.

What is expected?

Destroy should happen without any major delays.

What is actually happening?

Destroy takes 30+ seconds.

kevinlee11 avatar Jul 25 '22 05:07 kevinlee11

This sounds more like a Chromium/V8 bug, and likely a recent regression since we've never had similar issues in the past. I would suggest reporting this to Chromium/V8 instead since it's a pretty serious defect.

yyx990803 avatar Jul 26 '22 03:07 yyx990803

@yyx990803 Thanks for the quick response, logged an issue here: https://bugs.chromium.org/p/chromium/issues/detail?id=1347350.

For a temporary fix, not sure if you know off the top of your head if the arrays passed into the remove are dependent on order, and whether in our application's build process, we could modify the remove logic to use pop? Or even if we could offload some of the callers into a new popRemove function, it would help us immediately address the issue.

kevinlee11 avatar Jul 26 '22 05:07 kevinlee11

From your repro it took 20-30 seconds for me on Chrome, Firefox and Edge on windows (impressively edge was a bit faster)

Given that there is 10 000 elements would you say that 20 seconds is expected? Knowing that showing them took only 1 second, it's really weird that destroying takes so much longer

From my testing, this is not a browser specific issue, and it does seem normal that Arr.splice would take longer as all the elements after the removed position need to be moved in memory, so if there is 10 000 elements that need to be removed in 10 000 iterations it will obviously take a long long time, the problem being that the performance right now is O(n!) when it could clearly be a O(n) or less

It couldn't hurt to have those kind of performance improvements, since they're really not micro and this is a major bottleneck, this would give an overall performance improvement, so I'm really all for the addition of this unorderedRemove

Tofandel avatar Aug 31 '22 22:08 Tofandel

I think there is a perf issue with Chromium which really exasperates this particular issue, esp if there's a ton of watchers and computed props with lot of children elements (beyond my simple sample).

For a basic test I whipped this up: https://codesandbox.io/s/ih7t95?file=/src/index.js

And you'll notice it takes ~15-25 seconds on any Chromium browser vs ~0.05s on Firefox image image

Doesn't seem like Chromium will get around to solving that anytime soon, the bug I logged is still untriaged.

kevinlee11 avatar Sep 02 '22 13:09 kevinlee11

Okay now on this reproduction I'm getting the same result as you, weird that for the vue one even firefox takes 30+ seconds then

Tofandel avatar Sep 02 '22 14:09 Tofandel

Ya I think it's prob I just don't have a deep enough sample with fewer components but a lot more child components with a large number of watchers/computed props which is what causes this perf issue. In my team's actual prod site we only see the slow destroy on Chromium browsers.

kevinlee11 avatar Sep 02 '22 14:09 kevinlee11