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

Computed properties can have widely different performance characteristics on client and server (because they are not cached during SSR) #10151

Open
MoritzKn opened this issue Jun 13, 2019 · 4 comments

Comments

@MoritzKn
Copy link

MoritzKn commented Jun 13, 2019

Version

v2.4.3 - current (v2.6.10)

Reproduction link

https://jsfiddle.net/anc6Lf23/2/

Steps to reproduce

  1. Open the JSFiddle
  2. You will see the time it takes to compute a value without caching of computed properties (~1 second) in the output
  3. Set "CACHE = true" in the first line of the JS part to see the time it takes to compute the value with caching (~2 ms)

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 client
  • CACHE = false – the behavior during SSR

What 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

@LinusBorg
Copy link
Member

LinusBorg commented Jun 14, 2019

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:

  • Reactivity is disabled on the server for a reason.
  • Yet disabling it leads to stale caches for computed properties.
  • So disabling the cache for computed properties is technically necessary to ensure that the app shows the correct data.
  • The performance issues that you describe here are an ugly side effect of this.

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

  1. Improve SSR docs to raise awareness and
  2. maybe provide a helper function or something to use for memoizing computed props when on the server.

@MoritzKn
Copy link
Author

MoritzKn commented Jun 25, 2019

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.

@brophdawg11
Copy link

brophdawg11 commented Sep 14, 2020

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.

@marcel-dancak
Copy link

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
          }
        })
      })
    }
  }
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants