Improve Performance for Arrays that are not extensible (and therefore not reactive)

[fenduru]

Version

2.4.2

Reproduction link

https://jsfiddle.net/hgfx55h6/

Steps to reproduce

1. Create large array (in my case this array contains more Arrays)
2. Prevent extensions with Object.preventExtensions
3. Set the array on a Vue viewmodel

What is expected?

Vue's reactivity system should not do anything with this array, and setting this property should have the same runtime as setting a string on the viewmodel (i.e. O(1) )

What is actually happening?

The dependArray function is invoked recursively on the object every time the array is accessed, causing the setting of that property to take linear time. This can occur many times per tick.

---

This reproducer is very close to my actual scenario. I am building a virtual grid, where I have a large source array, and I compute some start index (normally this would be based on scroll position, in this case I've just added a button to simulate scrolling), and the template renders these rows.

In this jsfiddle, I have Object.preventExtensions()'d the array and never change it. The only thing I am changing is the start index, however doing so has an enormous amount of lag. When I profile this I see:

My issue at its core has nothing to do with reactivity, and is solely about performance. This does not seem like something that should perform this poorly.

[LinusBorg]

You seem to misunderstand what Object.createExtensions does.

The Object.preventExtensions() method prevents new properties from ever being added to an object (i.e. prevents future extensions to the object).

(from MDN, emphasis mine)

Vue doesn't add any new properties - it adds getters and setters for existing properties, which is fine with Object.preventExtensions.

Demo in vanilla Javascript: https://jsfiddle.net/Linusborg/ze47p6ta/

You probably want to rather use Object.freeze() or Object.seal()

[fenduru]

@LinusBorg perhaps this post should be titled differently, but please see the console timings in the reproducer I posted to see that there is clearly a bug in Vue, as it is recursively walking my array in an effort to make its elements reactive (which I do not want).

The following code should not take 17ms:

console.time('setting big array');
vm.arr = Object.preventExtensions(bigArray);
console.timeEnd('setting big array');

Vue does a similar thing with plain objects, however I can prevent this behavior with Object.preventExtension. I currently cannot prevent Vue from looping over all of the elements in an array though.

This is causing significant performance degradation in my project as I have a very large array that I need to be able to index into, but don't need to be reactive.

[fenduru]

Here is a simpler reproducer that doesn't involve Object.preventExtensions that takes >150ms to simply set a property to a large array.

https://jsfiddle.net/smh4fnms/

Edit:
And another one showing the hack I have to do currently: https://jsfiddle.net/h5ogrf1k/

[LinusBorg]

EDIT: The following is wrong, see my next reply.

You seem to misunderstand my explanation:

• Object.preventExtensions does not prevent reactivity, by design.
• Object.freeze does, by design.
• this is not a bug, it's an intentional design decision.

Solution to your problem: use Object.freeze

[LinusBorg]

I have to revert myself here, and apologize for claiming to be sure how it worked in this instance when in reality I was wrong, as it turns out.

In fact, we were both wrong: Vue does respect Object.preventExtensions.

You can test this out with your original jsfiddle. just log the array to the console after it was added to Vue, once with, once without Oject.preventExtensions().

You will notice that the one without the Object.preventExtensions() has an __ob__ property, which means it's now reactive, while the one with it doesn't.

I also checked this by adding breakpoints in vue.js on line 937 - when Object.preventExtensions() is defined, this is only called once (or twice, for some reason) - if it's not defined, it's called once for reach array element as the items are passed to the observe function by the reactivity system.

So apologies again for claiming something factually wrong, but this issue stays closed because Vue is already behaving like you want it to.

[fenduru]

Ah I see. I was surprised that there is a different way to prevent reactivity on an Object vs on an Array.

In Chrome Object.freeze/Object.preventExtensions is still taking 20ms to freeze an array with 100k items. It seems to depend on the length of the array.

Would it be possible to introduce a different way of indicating to Vue not to iterate this array? I do not actually need this object to be frozen, I am only doing so to change the behavior of Vue.

[fenduru]

Side note: The dependArray function is still getting invoked/the array iterated however I was mistaken in thinking this was the culprit of the performance issue

[LinusBorg]

Would it be possible to introduce a different way of indicating to Vue not to iterate this array?

That has been discussed a couple of times previously already, and won't happen.

If you really want to skip all reactivity, just add the array directly to the instance instead of assigning it to a property derived from data.

created() {
  this.bigArray = bigArray
}

[fenduru]

Okay, I'm going to take a step back because I think some of my initial debugging led me to think the problem was something that it wasn't.

This reproducer is very close to my actual scenario. I am building a virtual grid, where I have a large source array, and I compute some start index (normally this would be based on scroll position, in this case I've just added a button to simulate scrolling), and the template renders these rows.

In this jsfiddle, I have Object.preventExtensions()'d the array and never change it. The only thing I am changing is the start index, however doing so has an enormous amount of lag. When I profile this I see:

My issue at its core has nothing to do with reactivity, and is solely about performance. This does not seem like something that should perform this poorly.

I realize this issue has kind of gone off the rails from what I originally posted, so let me know if I should open a new issue.

[LinusBorg]

I'm not sure a new issue is in order - the behaviour with dependArray is not really something to work around within the reactivity functionality.

As I said, if you need raw performance and no reactivity, add the array as a normal instance propety:

https://jsfiddle.net/Linusborg/1s3hzsd5/

Further discussions about improving your specific performance challenges should probably rather happen on forum.vuejs.org, unless you have a specific improvement to suggest - that would warrant a new issue as a feature request.

[fenduru]

@LinusBorg with due respect, I find it really hard to believe that this is considered acceptable performance, especially considering the fact that I've marked the array as not extensible. The entire array (and sub-arrays) are being iterated every time the arr property is read off of the viewmodel.

In this reproducer, the template renders 10 rows, and each of these reads this.arr in the template expression {{ arr[start + i] }}. The problem is far worse if the number of rows is increased. The performance is basically O(n * m) where n = number of elements in the array, and m = number of times the array is referenced in your render function.

I believe this can be fixed by changing this line to:

if (Array.isArray(value) && Object.isExtensible(value)) {

I haven't been able to build/test due to #4338 however, so I'm not sure if this will break any tests, but I do believe this makes sense given the fact that Vue does respect isExtensible for non-array Objects.

[LinusBorg]

Good idea, but honestly, I'm not sure about the consequences.

@fenduru I reopened and rename the topic to better reflect the issue. Could you edit our OP with a comment linking down to where we got to the heart of the issue? thanks.

---

@yyx990803 You will have to chime in here ...

Evan, @fenduru is suggesting to keep dependArray from running in the getter here when the array isn't extensible. Could that have negative consequences?

I was thinking that this could interfere with reactive objects within the not-extensible array. Accessing a prop of such an object would trigger the object's own reactive getter, but only accessing the object as a whole (and e.g. passing it to a child component, or reading the keys of the object into an array with Object.keys() ...) might go unnoticed when skipping dependArray, right?

I'm not deep enough in this topic to say...

If we have to walk the array with dependArray, couldn't we maybe find a way that it's called only once per render cycle / Watcher instead of ntimes when the array is accessed n during render? The array can't change during render anyway...

Spontaneous idea, and I could be totally of the rails here:

1. After the first call of dependArray, add it to some prop like _cachedArrays of the watcher in Dep.target
2. For subsequent calls, make sure that the array is not in that cache already.
3. When popTarget in the watcher is called here, empty the cache.

// a Set would be much better for this...
if (Array.isArray(value) && (Dep.target._arrayCache.indexOf(value) === -1)) {
   dependArray(value)
   Dep.target._arrayCache.push(value)
}

// dep.js
export function popTarget () {
  Dep.target._arrayCache = null
  Dep.target = targetStack.pop(
}

Now the code has to walk the _arrayCache each time to check for presence of value, but I would assume that the number of array in a component will be much smaller then the length of the biggest array in a component, in most situations, so performance-wise, this would be an imporvement.

[jacelynfish]

According to your description of needing properties to be reactive at the top level, I think the real issue here is whether the data object or the array should be observed shallowly instead of whether Object.preventExtensions is called upon... In my opinion Object.preventExtensions only works as a mean to prevent further properties of being added to the object, while the original properties are still required to be reactive since they're likely to be referenced in templates. For instance even Object.preventExtensions is called on a specific array whose items are referenced in the templates, the changes of items inside this array still need to be demonstrated on the view layer. In this case dependArray being invoked recursively makes a lot of sense to me while this line does not... so I think what really matters here is figuring out a way to defined properties of the instance's data object in a shallow style.

[fenduru]

reading the keys of the object into an array with Object.keys() ...) might go unnoticed when skipping dependArray, right?

This behavior would be consistent with the behavior for Objects that have been preventExtensions'd -- indexing into the keys is not reactive, but if the values at the index is reactive everything will still work.

If we have to walk the array with dependArray, couldn't we maybe find a way that it's called only once per render cycle / Watcher instead of ntimes when the array is accessed n during render?

I was thinking about this as well - there is a performance hit for all applications right now, even ones that do want the reactivity because dependArray is called many times per tick. I suspect this could be reduced to once per tick in all cases, however in my case I am still advocating for avoiding calling it altogether when preventExtensions is used.

@jacelynfish To clarify what I mean by "reactive at the top level", I mean I only want the reactivity system to detect when the array reference itself has changed. My array is immutable, the only time any of its contents will change is when the array reference itself changes, so Vue being reactive on the arrays contents is a waste of performance for me.

[jacelynfish]

@fenduru but an array won't be immutable only by calling Object.preventExtensions 🤔

[fenduru]

@jacelynfish in my application I have guarantees that the array is immutable regardless of whether the object can technically be mutated.

[fenduru]

@LinusBorg @yyx990803 any thoughts on this? I would like to file a pull request for this however I am unable to get a working dev environment per #4338

[gggllm]

@jacelynfish @LinusBorg , @fenduru 's idea of using Object.preventExtensions will disable the array's reactiveness (no ob is added), therefore , the changes that happen to the elements will not be watched, so, the array being immutable or not, is not the issue here, right?

[yyx990803]

@fenduru sorry for the late response.

Yes, the way to get rid of this cost is by preventing dependArray when the Array is not extensible. However, as @LinusBorg pointed out, this does lead to the potential scenario where a frozen array containing objects that were already made reactive elsewhere - this would cause changes in those nested objects to be ignored. Although such cases should be extremely rare, I am not entirely sure if it's safe to sacrifice the correctness for the use case you mentioned.

Ultimately, dependArray is a somewhat brute-force workaround to deal with ES5's incapability of detecting Array indice access and Object property addition. (In the case of a[1].foo, if foo did not exist and was later added with Vue.set, dependArray is the only way to guarantee detection). Sadly in some cases it can be costly price to pay to ensure correctness.

Luckily, with the planned rewrite of the reactivity system using ES6 Proxy, we no longer need this work around and should see great perf boost in the scenarios you've been running into.

[fenduru]

@yyx990803 I can't replicate the behavior you are expecting in that case:

https://jsfiddle.net/3x1y6yg9/

I understand that dependArray is a brute-force workaround, however that seems like all the more reason to give people a way to opt out. This behavior is impacting every Vue component that references an array, and I imagine I am not the only person that has a relatively large array to index into.

I have opened #6467 and think it should be considered unless the scenario above can be shown to differ between objects and arrays (even then it seems kind of unreasonable for someone to explicitly opt out of change detection on an array and then be surprised when change detection doesn't happen within that array).

Edit: As a side note, while I think that ES6 Proxy is perfect for this kind of use case, it is not supported by IE11 and can't be polyfilled. IE is the most impacted browser for all things performance and is not going away any time soon, so I don't think we can ignore this issue in favor of ES6 proxies

[yyx990803]

@fenduru this is because you have one extra level of nesting, so the state object registers its dep.

See this case: https://jsfiddle.net/yyx990803/y8o98yjv/1/

[fenduru]

@yyx990803 I see, however this is actually an inconsistency between Objects and Arrays which is exactly my point. If you change that array to be an object that is frozen, the Vue.set is not recognized. I'm arguing that this makes sense, as you've indicated the object shouldn't be reactive.

https://jsfiddle.net/fzrj19zk/

[yyx990803]

@fenduru that's true. I guess for now it's actually better to keep them consistent.