From 495a969160cb7a26d768dfd492e5bddf160adf42 Mon Sep 17 00:00:00 2001 From: Michael Best Date: Thu, 29 Dec 2016 09:54:27 -0800 Subject: [PATCH] when using deferred updates, only evaluate a computed if one of its dependencies actually changes (rather than just being "dirty") --- spec/asyncBehaviors.js | 20 ++++++++++++++ src/subscribables/dependentObservable.js | 34 +++++++++++++++++------- src/subscribables/subscribable.js | 2 +- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/spec/asyncBehaviors.js b/spec/asyncBehaviors.js index f34593aff..452340bf3 100644 --- a/spec/asyncBehaviors.js +++ b/spec/asyncBehaviors.js @@ -975,6 +975,26 @@ describe('Deferred', function() { expect(notifySpy.argsForCall).toEqual([['i(x,h(cx,g(ex,fx),d(bx,cx)),bx,fx)']]); // only one evaluation and notification }); + it('Should minimize evaluation when dependent computed doesn\'t actually change', function() { + // From https://github.com/knockout/knockout/issues/2174 + this.restoreAfter(ko.options, 'deferUpdates'); + ko.options.deferUpdates = true; + + var source = ko.observable({ key: 'value' }), + c1 = ko.computed(function () { + return source()['key']; + }), + countEval = 0, + c2 = ko.computed(function () { + countEval++; + return c1(); + }); + + source({ key: 'value' }); + jasmine.Clock.tick(1); + expect(countEval).toEqual(1); + }); + it('Should ignore recursive dirty events', function() { // From https://github.com/knockout/knockout/issues/1943 this.restoreAfter(ko.options, 'deferUpdates'); diff --git a/src/subscribables/dependentObservable.js b/src/subscribables/dependentObservable.js index df954b2dd..81d704582 100644 --- a/src/subscribables/dependentObservable.js +++ b/src/subscribables/dependentObservable.js @@ -18,6 +18,7 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva var state = { latestValue: undefined, isStale: true, + isDirty: false, isBeingEvaluated: false, suppressDisposalUntilDisposeWhenReturnsFalse: false, isDisposed: false, @@ -45,7 +46,7 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva } else { // Reading the value ko.dependencyDetection.registerDependency(computedObservable); - if (state.isStale || (state.isSleeping && computedObservable.haveDependenciesChanged())) { + if (state.isStale || state.isDirty || (state.isSleeping && computedObservable.haveDependenciesChanged())) { computedObservable.evaluateImmediate(); } return state.latestValue; @@ -166,16 +167,19 @@ var computedFn = { markDirty: function () { // Process "dirty" events if we can handle delayed notifications if (this._evalDelayed && !this[computedState].isBeingEvaluated) { - this._evalDelayed(); + this._evalDelayed(false /*isChange*/); } }, isActive: function () { - return this[computedState].isStale || this[computedState].dependenciesCount > 0; + var state = this[computedState]; + return state.isStale || state.isDirty || state.dependenciesCount > 0; }, respondToChange: function () { // Ignore "change" events if we've already scheduled a delayed notification if (!this._notificationIsPending) { this.evaluatePossiblyAsync(); + } else if (this[computedState].isDirty) { + this[computedState].isStale = true; } }, subscribeToDependency: function (target) { @@ -202,7 +206,7 @@ var computedFn = { computedObservable.evaluateImmediate(true /*notifyChange*/); }, throttleEvaluationTimeout); } else if (computedObservable._evalDelayed) { - computedObservable._evalDelayed(); + computedObservable._evalDelayed(true /*isChange*/); } else { computedObservable.evaluateImmediate(true /*notifyChange*/); } @@ -322,7 +326,7 @@ var computedFn = { ko.utils.objectForEach(dependencyDetectionContext.disposalCandidates, computedDisposeDependencyCallback); } - state.isStale = false; + state.isStale = state.isDirty = false; } }, peek: function () { @@ -336,15 +340,26 @@ var computedFn = { limit: function (limitFunction) { // Override the limit function with one that delays evaluation as well ko.subscribable['fn'].limit.call(this, limitFunction); - this._evalDelayed = function () { + this._evalIfChanged = function () { + if (this[computedState].isStale) { + this.evaluateImmediate(); + } + return this[computedState].latestValue; + }; + this._evalDelayed = function (isChange) { this._limitBeforeChange(this[computedState].latestValue); - this[computedState].isStale = true; // Mark as dirty + // Mark as dirty + if (isChange) { + this[computedState].isStale = true; + } else { + this[computedState].isDirty = true; + } - // Pass the observable to the "limit" code, which will access it when + // Pass the observable to the "limit" code, which will evaluate it when // it's time to do the notification. this._limitChange(this); - } + }; }, dispose: function () { var state = this[computedState]; @@ -361,6 +376,7 @@ var computedFn = { state.dependenciesCount = 0; state.isDisposed = true; state.isStale = false; + state.isDirty = false; state.isSleeping = false; state.disposeWhenNodeIsRemoved = undefined; state.disposeWhen = undefined; diff --git a/src/subscribables/subscribable.js b/src/subscribables/subscribable.js index 13ca67418..d990f3729 100644 --- a/src/subscribables/subscribable.js +++ b/src/subscribables/subscribable.js @@ -104,7 +104,7 @@ var ko_subscribable_fn = { // If an observable provided a reference to itself, access it to get the latest value. // This allows computed observables to delay calculating their value until needed. if (selfIsObservable && pendingValue === self) { - pendingValue = self(); + pendingValue = self._evalIfChanged ? self._evalIfChanged() : self(); } ignoreBeforeChange = false; if (self.isDifferent(previousValue, pendingValue)) {