Ability to disable reactivity to improve performance on destroy

[emarbo]

What problem does this feature solve?

Destroying N RouterLink at once has a cost of O((N^2)/2) due to all RouterLink depend on the singleton reactive property $route. When removing hundreds or thousands of links from the page, this cost is noticeable, especially on slow devices. The goal is a cost of O(N).

Hundreds or thousands of links might seem too much, but sure there might be some cases where it makes sense, like an infinite scroll or some big grids. In many cases vue-virtual-scroller should solve the general problem, but I think it would be better not to require it as a rule.

Removing the dependency on $route means effectively disabling reactivity for the RouterLink and thus all the related features. At this moment, I think the only affected features are the CSS classes to show a link is active.

Why this cost?

The singleton _route is created for the _routerRoot once. At this moment, Vue instantiates a Dep to track writes and reads on this attribute.

Each time a RouterLink is created, its render Watcher detects the $route is accessed, so that the Watcher adds the Dep to its dependencies (Watcher.deps) and the Dep adds the Watcher to its subscribers (Dep.subs). This means the Dep.subs for _route is, at least, as large as the number of rendered RouterLinks.

When the $route changes, all the RouterLink are updated because they are on the Dep.subs.

When destroying all the RouterLink (page change), all the components teardown their watchers, and each watcher have to remove itself from the Dep.subs. Removing a single RouterLink's Watcher from the Dep.subs costs O(N - i), where N is the number of RouterLink and i the number of RouterLink already removed.

Generally speaking, the cost of removing N RouterLinks at once is O((N^2)/2).

What does the proposed API look like?

Adding a no-reactive prop on RouterLink should be enough.

The meaning of this prop is effectively disabling reactivity on the current route to improve the overall performance, especially when destroying many RouterLink at once.

As a side effect, this flag turns off any feature that depends on the current route. At this moment, I think the only affected features are related to the CSS classes to show if a link is active or not.

[posva]

This is a really nice analysis of the existing problem. Thank you for that!
I think that if one does not care about active classes, you can create a custom router link that calls router.resolve() for the href and then calls push when clicked. I will give it a shot tomorrow. I don't think adding a prop is the right api choice here because this is still and edge case. Maybe a new component could help. Definitely worth of an RFC!

[emarbo]

Thank you @posva! We actually did the performance tests using a custom router link, as you suggested. However, there are two points I don't feel 100% comfortable with this solution:

• We had to copy the guardEvent function to keep the same behaviour on click (the function is not exported)
• We are using NuxtLink, which extends the RouterLink. If we wanted to keep the Nuxt prefetch behaviour, we would have to copy part of their code.

On the whole, a custom router link is harder to maintain, and you probably end up picking desired features from the original source with the risk of doing something wrong on the way. This is a point for including a flag in the RouterLink itself though it could not be justified if it doesn't affect many projects.

Regarding the impact, I think it might affect more projects than expected because the issue is not easy to detect and so report. We detected it because we were analyzing all Dep with a subs larger than 400 when rendering more than 400 user cards (requires modifying the vue runtime). In other words, analyzing all Dep which subs grew linearly for each user card on our infinite scroll. This is something you don't do in your day by day xD. When inspecting the DevTools, you only see many many calls to $destroy, invokeDestroyHook, teardown, removeSub and remove, which means nothing to you because all of them are part of the Vue runtime. The difficult part is finding who formerly caused the large Dep.subs. This discouraged me a few times until I have found the days and inspiration to understand reactivity better and find the cause.

[wparad]

We are using NuxtLink, which extends the RouterLink. If we wanted to keep the Nuxt prefetch behaviour, we would have to copy part of their code.

This just causes a leaky abstraction then. A feature which isn't frequently necessary, which is wrapped, and then wrapped, etc... Will force all the wrappers to implement similar functionality. A better solution here would be for the NuxtLink to accept a <slot> which you could pass in your custom RouterLink component.

We had to copy the guardEvent function to keep the same behaviour on click (the function is not exported)
If this offers value in not needing to be copied, then what's the feasibility for vue-router to export it?

Perhaps that's a better solution, perhaps there is a change that vue-router could implement instead, but just looking at the suggested solution there are problems with it:

• The solution to add the flag isn't discoverable. I as a developer don't want to have to do the same amount of investigating your went through all to realize this problem, find this github issue, with the solution being add a magic attribute flag to the component.
• Going the other way similarly a problem, since the feature is not common, the extra flag adds unnecessary complexity for standard use of the component.
• All the wrappers of RouterLink also need to consider how to expose this same attribute if makes sense for them. So for non-nuxt use cases, is this also a problem? Or is this one implementation where the issue lies.

It would be good to understand the feasibility to make a change to the NuxtLink, since that seems more likely where developers would get the benefit. Although it's possible that I'm missing something.

[posva]

Each time a RouterLink is created, its render Watcher detects the $route is accessed, so that the Watcher adds the Dep to its dependencies (Watcher.deps) and the Dep adds the Watcher to its subscribers (Dep.subs). This means the Dep.subs for _route is, at least, as large as the number of rendered RouterLinks.

I don't think this can change within RouterLink without major code complexity. Which isn't worth for an edge case. I rather add a new component and document it. Duplicating guardEvent() is really fine for this scenario.

Note you can also use v-once if your router-links do not render ever again. It should help.

[emarbo]

@wparad:

This just causes a leaky abstraction then. A feature which isn't frequently necessary, which is wrapped, and then wrapped, etc... Will force all the wrappers to implement similar functionality. A better solution here would be for the NuxtLink to accept a which you could pass in your custom RouterLink component.

The main purpose of NuxtLink is prefetching the Components required for the new route, so the JS is already fetched if the user clicks on the link. They extend the RouterLink and read the to (prop) for doing the job. I think using a <slot> wouldn't fit for this purpose.

As they extend the RouterLink, they don't have to expose anything from RouterLink; the interface is exactly the same but extended with the Nuxt functionality.

The solution to add the flag isn't discoverable

I don't see why it isn't discoverable. Maybe, I didn't explain correctly. By flag, I meant a prop like exact-path that should be documented explaining the use case and the side effects (disables active classes).

It would be good to understand the feasibility to make a change to the NuxtLink, since that seems more likely where developers would get the benefit. Although it's possible that I'm missing something.

The root cause is the dependency that RouterLink has on the $route (accesses it in the render). In fact, the NuxtLink implementation does not depend on the $route (reactively speaking). As far as I understand, the benefit goes the other way around: if we solve the issue at RouterLink, it is beneficial for any extending component.

@wparad, @postva:

Going the other way similarly a problem, since the feature is not common, the extra flag adds unnecessary complexity for standard use of the component.

I don't think this can change within RouterLink without major code complexity.

I see what you mean. In my opinion, the current change is quite simple, but the complexity will grow if the new features to come depend on the $route.

Having two components to maintain has pros and cons. The main benefit is the RouterLink is kept as it is. The downside is there will be two RouterLink-like components to maintain in the vue-router. Moreover, NuxtLink-like components can't extend from both implementations, so they shall stick to one or provide two components, one for each vue-router implementation.

All in all, I don't see the point of having the two components in the official vue-router for an edge case. I'd rather go with the no-reactivity flag if the complexity doesn't grow that much. Alternatively, I can just go with a custom implementation for these edge cases - in the end, we just needed the guardEvent and a bit of code as described by first @posva comment.

Note you can also use v-once if your router-links do not render ever again. It should help.

I tried it with no luck. It seems that the v-once creates the component but it doesn't destroy it, so watchers aren't removed until the component is removed from the DOM.

Many thanks for the support! I hope I hadn't missed any point.

[wparad]

I don't see why it isn't discoverable. Maybe, I didn't explain correctly. By flag, I meant a prop like exact-path that should be documented explaining the use case and the side effects (disables active classes).

Let me try this differently, how do I as a developer using vue-router know that I should add this flag. From the expectation of functionality, it doesn't seem like something anyone would intentionally set until they had a problem. And in that case, how do they know that the problem could be solved by adding the flag. For good usability, we should have an answer to that.

Additionally, I think it is bad form to explicitly disable the functionality and thus break it for supporting applying the css changes. I'm not sure what the long term goal is, but basically it would be better to ask, does it make sense that it does this? If the answer is yes, then adding a property to disable this functionality, seems like the wrong approach. If the answer is: well, no, it likely shouldn't be doing this actually, then the change in the interim while the functionality is being removed is a good short term solution. So knowing the long term plans is critical to making that judgment.

Removing a single RouterLink's Watcher from the Dep.subs costs O(N - i), where N is the number of RouterLink and i the number of RouterLink already removed.

This actually seems where the bug is more than anything. Why does removing a single RouterLink's Watcher cost O(N - i)? I mean actually in big O notation that's just O(n) why is removing a watch O(n) and not just O(1)? Where's that cost coming from? That's probably the best place to start investigation, if we do that then it would resolve the same problem in any other place where we frequently remove a large number of watchers due to components being destroyed. While uncommon, still likely to happen and cause a similar problem.

[emarbo]

@wparad, thanks for the clarification! I see what you mean and I think you're right.
It might be better solutions than adding a flag for this purpose:

Change Dep.subs implementation: use a Set instead of an Array. This would reduce the cost of deleting n RouterLink at once to O(n) (instead of the O(n^2)). I have just checked the Vue 3 implementation and seems they are already using Sets. This means Vue 3 users shouldn't be affected by this issue! 🎉

Use browser's native API. The point of watching the $route is adding/removing classes to the link when the URL changes. IIUC, RouterLink could archive the same by listening to the popstate event.
I don't know which data structure browsers use for holding the event's listeners, but it must be faster to call N times to Window.removeEventListener (having N listeners), than calling N times to Array.indexOf + Array.splice (having N elements). Sure browsers use some kind of Map internally so removing a single listener is O(1).

I suppose the first option isn't worth it because it's at the heart of the Vue 2 dependency system and it's already fixed on Vue 3. The second change would help projects still using Vue 2 although, if it's already fixed on Vue 3, does it make sense to change the RouterLink implementation now?

This actually seems where the bug is more than anything. Why does removing a single RouterLink's Watcher cost O(N - i)? I mean actually in big O notation that's just O(n) why is removing a watch O(n) and not just O(1)? Where's that cost coming from?

The main point is that the cost of removing a Watcher from a Dependency (Dep) is the cost of removing an element from the array Dep.subs (O(n)). Being n the number of RouterLink, there are n Watchers on the Dep.subs corresponding to the $route. When removing the all n RouterLink at once, it implies n array removals on the same array, so the O(n^2) (the O((n^2)/2) is more accurate but just a detail).
Please, have a look at the issue description for an extended explanation with links to the code. It's a bit difficult to explain using words and probably easier to understand following the code 😅.

[emarbo]

Hello all, I will close the issue since Vue 3 seems to have solved the problem internally because they have replaced the subscribers array with a set, and the set has constant performance for deletion.

I don't have time to test it now, but it should really help. Here it is a very similar problem in vue-i18n and the performance after applying this little change. In their case, they don't use Vue internals to track the reactivity on language changes, but the underlying problem was exactly the same.

[bryarb]

Posted a bounty here: https://app.bountysource.com/issues/97354830-ability-to-disable-reactivity-to-improve-performance-on-destroy
I need this issue fixed for 2.6.* , not 2.7.* since I would need to rewrite 90% of my site to change versions