Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An anti-pattern in computed property may cause performance issue #8728

Open
Yuyz0112 opened this issue Aug 29, 2018 · 1 comment
Open

An anti-pattern in computed property may cause performance issue #8728

Yuyz0112 opened this issue Aug 29, 2018 · 1 comment
Labels

Comments

@Yuyz0112
Copy link

Yuyz0112 commented Aug 29, 2018

Version

2.5.17

Reproduction link

https://codesandbox.io/s/140202wlq

Steps to reproduce

Open the code sandbox link and click the button to use the computed property 'filteredEntities', and you will find it cost a lot of time.

After check the call tree, I found this is because the code calls the getter of 'entities' many times which will then call the dependArray function.

The code leading to this is:

const len = this.entities.length;
for (let i = 0; i < len; i++) {
  const e = this.entities[i];
  // do something with e
}

If this.entities has a length of n, its getter will be called n + 1 times. And every time when the getter was called, it will call dependArray on the value of this.entities, which is the array with length n.
Since dependArray will iterate the value, the depend function will be called (n + 1)^2 times totally.

I found this code when I'm reviewing someone's PR, and the original code use for (let i = 0; i < this.entities.length; i++) and make another n times call to the getter.

It was easy to avoid this problem, such as using some array methods like filter to only call getter once.
Even cache the value of entities by const cache = this.entities can solve it.

Although it was easy to avoid of this pattern, I still think it is dangerous because the original code was not an obvious anti-pattern.
And this will not show any performance issue when the array is small, but may cause serious performance issue when the array is large in some production environment.

What is expected?

I've read the related source code and I think this is the expected result of the observer system.

Then I read some chapter of the guide again to ensure there is no mention about this, so I'm not sure whether we need to add this as a NOT TODO in the computed property chapter.

What is actually happening?

Described.

@plainnany
Copy link

Is this issue going to be fixed in vue version 2.+ ?

richard-oven-vanti added a commit to vanti-dev/sc-bos that referenced this issue Feb 4, 2024
Fixed a coding pattern which has had a huge performance impact on the energy graph.
See: vuejs/vue#8728

Jira: N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants