From 3d99c96f532d37f1ac2144b5b68165916d443546 Mon Sep 17 00:00:00 2001 From: loganlinn Date: Wed, 6 May 2015 11:12:26 -0700 Subject: [PATCH 1/3] Evaluate stale getter deps only once Move logic around to avoid evaluating dependencies twice in certain code path --- src/evaluator.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/evaluator.js b/src/evaluator.js index 90bb117..b6f79a7 100644 --- a/src/evaluator.js +++ b/src/evaluator.js @@ -53,22 +53,22 @@ class Evaluator { // Cache hit return this.__cachedGetters.getIn([code, 'value']) - } else if (this.__hasStaleValue(state, keyPathOrGetter)) { + } + + // evaluate dependencies + var args = getDeps(keyPathOrGetter).map(dep => this.evaluate(state, dep)) + + if (this.__hasStaleValue(state, keyPathOrGetter)) { + // getter deps could still be unchanged since we only looked at the unwrapped (keypath, bottom level) deps var prevValue = this.__cachedGetters.getIn([code, 'value']) var prevArgs = this.__cachedGetters.getIn([code, 'args']) - // getter deps could still be unchanged since we only looked at the unwrapped (keypath, bottom level) deps - var currArgs = toImmutable(getDeps(keyPathOrGetter).map(getter => { - return this.evaluate(state, getter) - })) // since Getter is a pure functions if the args are the same its a cache hit - if (isEqual(prevArgs, currArgs)) { + if (isEqual(prevArgs, args)) { this.__cacheValue(state, keyPathOrGetter, prevArgs, prevValue) return prevValue } } - // no cache hit evaluate - var args = getDeps(keyPathOrGetter).map(dep => this.evaluate(state, dep)) // This indicates that we have called evaluate within the body of a computeFn. // Throw an error as this will lead to inconsistent caching From aeeb00d557696c44787a2cb22b0809f8b2ecdbfa Mon Sep 17 00:00:00 2001 From: loganlinn Date: Wed, 6 May 2015 13:29:26 -0700 Subject: [PATCH 2/3] Move prevValue into block where needed --- src/evaluator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evaluator.js b/src/evaluator.js index b6f79a7..13305e3 100644 --- a/src/evaluator.js +++ b/src/evaluator.js @@ -60,11 +60,11 @@ class Evaluator { if (this.__hasStaleValue(state, keyPathOrGetter)) { // getter deps could still be unchanged since we only looked at the unwrapped (keypath, bottom level) deps - var prevValue = this.__cachedGetters.getIn([code, 'value']) var prevArgs = this.__cachedGetters.getIn([code, 'args']) // since Getter is a pure functions if the args are the same its a cache hit if (isEqual(prevArgs, args)) { + var prevValue = this.__cachedGetters.getIn([code, 'value']) this.__cacheValue(state, keyPathOrGetter, prevArgs, prevValue) return prevValue } From 7534f03903f4aecab60033e3e3806a50d996f7c5 Mon Sep 17 00:00:00 2001 From: loganlinn Date: Mon, 11 May 2015 18:23:17 -0700 Subject: [PATCH 3/3] Fix evaluator's deps comparison: use Immutable val --- src/evaluator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evaluator.js b/src/evaluator.js index fbcdbac..eb32f62 100644 --- a/src/evaluator.js +++ b/src/evaluator.js @@ -63,7 +63,7 @@ class Evaluator { var prevArgs = this.__cachedGetters.getIn([code, 'args']) // since Getter is a pure functions if the args are the same its a cache hit - if (isEqual(prevArgs, args)) { + if (isEqual(prevArgs, toImmutable(args))) { var prevValue = this.__cachedGetters.getIn([code, 'value']) this.__cacheValue(state, keyPathOrGetter, prevArgs, prevValue) return prevValue