Open MoritzKn opened 5 years ago
I think you are aware of this (judging from how deep you digged into this), but I want to quickly re-iterate so people are on the same page:
So technically it's not a bug as it "works as designed", but the end result is not satisfactory, of course. And there's no straightforward solution to this either, as far as I can tell at least.
The best I can think of right now would be to
I think your proposed changes would be good improvements but I'm wondering if we can not go a bit further.
I would even question if the performance advantage of disabling reactivity on the server pays off, considering that that means computed properties aren't cached. It would be interesting to get some numbers on that.
But maybe there is a way to cache computed properties without having stale caches: As far as I understand, caching for computed properties was only disabled because the Vuex state changes during SSR and getters then return outdated results. Though, in the Vuex case, we don't need the whole reactivity system to tell if the state changed because all state changes are encapsulated in mutations. So perhaps Vuex could just "rest" the computed cache for the getters after a mutation.
This would mean that the Vue.js Core had to provide a new API for "resetting" computes on the server. And Vuex would need to trigger that API after a mutation.
To @LinusBorg's second bullet, we ran into this recently and I wanted to drop in our optimization workaround. We had a single getter that was massaging some data we get back from an API (in a particularly unuseful data structure) and the getter was converting the data into a much simpler data structure to be used in components. However, computing the simpler structure took a triple-nested loop that could take close to 1ms - and that method could 50+ times during a single SSR page load because it was accessed in a bunch of places.
One option would have been to lift the getter access to a parent component and pass it via a prop all the way down - however, the component tree is fairly large and that would have been a pretty invasive change at the moment. Another would be to move this computation out of the UI and offload to the api, but that would require a pretty significant refactor to the app as well.
So instead we decided to pre-compute the value during SSR and store it in the store state
, but since it's derivative data of other data in state
we didn't want to increase the size of the stringified state
sent to the client. So we used a custom JSON.stringify
replacer function to this key when stringifying the state
.
The end code looked something like:
// store.js
return new Vuex.Store({
state: {
apiData: null,
_ssr_uiMap: null,
},
mutations: {
setApiData(state, apiData) {
state.apiData = apiData;
},
setUiMap(state, uoMap) {
state._ssr_uiMap = uiMap;
},
},
actions: {
loadApiData({ commit }) {
const apiData = await loadApiData();
commit('setApiData', apiData);
if (process.env.VUE_ENV === 'server') {
commit('setUiMap', computeUiMap(apiData));
}
},
},
getters: {
uiMap(state) {
if (process.env.VUE_ENV === 'server' && state._ssr_uiMap) {
return state._ssr_uiMap
}
return computeUiMap(state.apiData);
},
},
});
// And then when we stringify the store in entry-server.js, we use the following
// to strip any _ssr_ prefixed keys from the state
JSON.stringify(store.state, (k, v) => k.startsWith('_ssr_') ? undefined : v)
So we can memoize the value on the server and only calculate it once, while still leveraging the built-in getter caching client side without impacting payload size of the stringified store.
It's not a 100% ideal solution, but it works quite well in limited cases and we saw around a 20-25% increase in SSR render time.
I have also experienced significant performance issues in SSR mode due to this behaviour. Even if I have later found and optimized the main bottleneck, I still don't like such greatly different behaviour of computed properties in SSR mode. That's why I tried to find en easy way to selectively enable caching of computed properties in components, where it is safe to do. Here is my solution, which seems to work fine for me, so maybe it could be also useful for others.
My idea was to add support for a new option, which will enable/control caching of computed properties on server.
// Vue component
export default {
computed: {
compA () {},
compB () {}
},
ssrComputedCache: true, // enable caching for all computed properties
ssrComputedCache: ['compA'] // or enable cache for selected computed properties
}
And here is a global mixin implementing this feature, which can be used in nuxt app as plugin
// nuxt plugin (mode: 'server')
import Vue from 'vue'
Vue.mixin({
created () {
if (process.server && this.$options.ssrComputedCache) {
const { computed, ssrComputedCache } = this.$options
const cachedProperties = ssrComputedCache === true ? Object.keys(computed) : ssrComputedCache
cachedProperties.forEach(name => {
let value
let cached = false
Object.defineProperty(this, name, {
// for repeated re-definition in dev mode, depending on runInNewContext setting
// (https://github.com/nuxt/nuxt.js/issues/7171)
configurable: true,
get () {
if (cached) {
return value
}
value = computed[name].apply(this)
cached = true
return value
}
})
})
}
}
})
Version
v2.4.3
- current (v2.6.10
)Reproduction link
https://jsfiddle.net/anc6Lf23/2/
Steps to reproduce
This issue concerns SSR but for simplicity I created the fiddle which emulates the behavior of the server render:
CACHE = true
– the behavior we usually have in the clientCACHE = false
– the behavior during SSRWhat is expected?
I would expect computed properties to have comparable performance characteristics on the server and client, so that I don't need to write custom code. I.e. I would expect computed properties to be cached during SSR.
What is actually happening?
Computed properties are not cached and therefore have drastically different performance characteristics in some cases.
Description if the issue
Since computed properties are not cached during SSR some components unexpectedly take significantly longer to render. This is the case if it is heavy to compute the property or if it is accessed a lot.
I would usually expect a computed property to have constant time complexity (
O(1)
) no matter how often we access it. But on the server it suddenly becomes linear time complexity (O(n)
). This is especially critical when the computed is accessed in a loop. When we have multiple computed properties relaying on each other, each containing loops, this gets really bad. Then it has polynomial time with the exponent being the amount of nested computed properties (E.g.O(n^3)
for three levels of computes, like in the JSFiddle)Real world example
I noticed this issue because our server renderer suddenly took multiple seconds (5-8 seconds) to respond after enabling a new component for SSR. Normally rendering the app on the server takes about 100ms.
The effected component is part of a proprietary code base but it is similar to the one in the JSFiddle. You can see the component in production here:
After finding out about this I also investigated other occurrences of this: In our Vuex store we did not have any nested getters with loops, like described above, however some of the getters are somewhat heavy to compute (~1ms) and accessed a lot in various templates. So I decided to introduce a very simple custom caching layer to our store. This sped up server rendering by about 20%.
This could also be a low hanging fruit for optimizing SSR performance: Based on analyzing our own app, I would roughly estimate that caching all computed properties could speed up server rendering by about 30% in an average Vue.js app.
For me this issue was hard to understand because the affected code seemed harmless at first.
Mitigation
To mitigate this issue you can move access to computes out of loops: access them once, store them in a local variable and then use this variable in the loop. This is generally a good idea since any look up of a property on a Vue.js VM has a small cost.
However this is not possible if you have a loop inside your templates.
References
The code that controls this behavior: https://github.com/vuejs/vue/blob/0948d999f2fddf9f90991956493f976273c5da1f/src/core/instance/state.js#L208-L219
The issue that lead to this behavior being introduced: vuejs/vuex#877
The commit that introduced this behavior 06741f32
Earlier occasion of someone stumbling over this: