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

ComputedValue.get not optimal #266

Closed
dettier opened this issue May 26, 2016 · 5 comments
Closed

ComputedValue.get not optimal #266

dettier opened this issue May 26, 2016 · 5 comments

Comments

@dettier
Copy link

dettier commented May 26, 2016

Hi everyone!
I believe something is not quite optimal inside ComputedValue.get function. Computed function gets re-evaluated when there's really no need for it.

Here's an example, every access to heavyComputed takes 1 second:

const extendObservable = require('mobx').extendObservable;

const obj = {};
extendObservable(obj, {
  obs : 42,
  heavyComputed : () => {
    const start = Date.now();
    while(Date.now() - start < 1000) {};
    return obj.obs;
  }
});

console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
@mattruby
Copy link
Member

If you have that field in an autorun, it should cache as you'd like.
On May 26, 2016 2:27 PM, "Sergey" notifications@github.com wrote:

Hi everyone!
I believe something is not quite optimal inside ComputedValue.get
function. Computed function gets re-evaluated when there's really no need
for it.

Here's an example, every access to heavyComputed takes 1 second:

const extendObservable = require('mobx').extendObservable;

const obj = {};
extendObservable(obj, {
obs : 42,
heavyComputed : () => {
const start = Date.now();
while(Date.now() - start < 1000) {};
return obj.obs;
}
});

console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
console.log(obj.heavyComputed);
console.log(obj.heavyComputed);


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#266

@mweststrate
Copy link
Member

@dettier Indeed, the heaveComputed is not caching because you are not using it in your application (as far as MobX knows). As soon as you use the computed value in a React component, or do the logging in an autorun like autorun(() => console.log(obj.heavyComputed) you will see different behavior.

The reason for this is that MobX automatically detect whether something is depending on your heavyComputed. If this isn't the case, MobX will automatically suspend the computation. This means that you don't have to manually dispose these computed values. So as long as you use the value in your UI, the value will stay up to date, but as soon as you dismount the component, MobX knows that the computed value is not relevant for your application anymore, and won't waste any CPU cycles on trying to keep it up to date. Because one application might have millions of computed values, but it is not likely that millions of components are listening to all those values. So in practice this is a very smart strategy to apply.

The result of that is that MobX needs to fall back to normal eager evaluation if a computation has suspended and you 'suddenly' still request for it. That is why you see the inefficiency.

@AriaFallah
Copy link

@mweststrate

The result of that is that MobX needs to fall back to normal eager evaluation if a computation has suspended and you 'suddenly' still request for it

Would createTransformer fix this behavior? Just curious how you would avoid it without using it in an autorun/react.

@mweststrate
Copy link
Member

Nope, createTransformer still needs an observer. Any means of observing is enough to keep it alive, even if you do not use it in the observer, but somewhere outside, just autorun(() => obj.heavyComputed) or observe(obj, 'heavyComputed', () => {}) is enough to keep it alive.

But in practice I never encountered a use case where I needed this. Why should data be kept up-to-data if nobody needs it? Always try to postpone a computation to the latest responsible moment.

@dettier
Copy link
Author

dettier commented May 27, 2016

@mweststrate Thanks for the clarification! I encountered this when implementing debounced computed with immediate=true behaviour and the call to get initial value indeed wasn't inside autorun or other dep tracking context. autorun fixes this.

Thanks again for detailed explanation.

@dettier dettier closed this as completed May 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants