From 45b87b293f1ac8fae3b61eb0b88a083f1d55b90e Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Tue, 3 Dec 2013 19:16:08 -0500 Subject: [PATCH] chore(Scope): short-circuit after dirty-checking last dirty watcher Stop dirty-checking during $digest after the last dirty watcher has been re-checked. This prevents unneeded re-checking of the remaining watchers (They were already checked in the previous iteration), bringing a substantial performance improvement to the average case run time of $digest. Closes #5272 Closes #5287 --- src/ng/rootScope.js | 53 +++++++++++++++++++--------- test/ng/rootScopeSpec.js | 76 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 17 deletions(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index a54fdc98fc1e..233dfa3b2f60 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -71,6 +71,7 @@ function $RootScopeProvider(){ var TTL = 10; var $rootScopeMinErr = minErr('$rootScope'); + var lastDirtyWatch = null; this.digestTtl = function(value) { if (arguments.length) { @@ -325,6 +326,8 @@ function $RootScopeProvider(){ eq: !!objectEquality }; + lastDirtyWatch = null; + // in the case user pass string, we need to compile it, do we really need this ? if (!isFunction(listener)) { var listenFn = compileToFn(listener || noop, 'listener'); @@ -553,6 +556,8 @@ function $RootScopeProvider(){ beginPhase('$digest'); + lastDirtyWatch = null; + do { // "while dirty" loop dirty = false; current = target; @@ -565,8 +570,10 @@ function $RootScopeProvider(){ clearPhase(); $exceptionHandler(e); } + lastDirtyWatch = null; } + traverseScopesLoop: do { // "traverse the scopes" loop if ((watchers = current.$$watchers)) { // process our watches @@ -576,22 +583,30 @@ function $RootScopeProvider(){ watch = watchers[length]; // Most common watches are on primitives, in which case we can short // circuit it with === operator, only when === fails do we use .equals - if (watch && (value = watch.get(current)) !== (last = watch.last) && - !(watch.eq - ? equals(value, last) - : (typeof value == 'number' && typeof last == 'number' - && isNaN(value) && isNaN(last)))) { - dirty = true; - watch.last = watch.eq ? copy(value) : value; - watch.fn(value, ((last === initWatchVal) ? value : last), current); - if (ttl < 5) { - logIdx = 4 - ttl; - if (!watchLog[logIdx]) watchLog[logIdx] = []; - logMsg = (isFunction(watch.exp)) - ? 'fn: ' + (watch.exp.name || watch.exp.toString()) - : watch.exp; - logMsg += '; newVal: ' + toJson(value) + '; oldVal: ' + toJson(last); - watchLog[logIdx].push(logMsg); + if (watch) { + if ((value = watch.get(current)) !== (last = watch.last) && + !(watch.eq + ? equals(value, last) + : (typeof value == 'number' && typeof last == 'number' + && isNaN(value) && isNaN(last)))) { + dirty = true; + lastDirtyWatch = watch; + watch.last = watch.eq ? copy(value) : value; + watch.fn(value, ((last === initWatchVal) ? value : last), current); + if (ttl < 5) { + logIdx = 4 - ttl; + if (!watchLog[logIdx]) watchLog[logIdx] = []; + logMsg = (isFunction(watch.exp)) + ? 'fn: ' + (watch.exp.name || watch.exp.toString()) + : watch.exp; + logMsg += '; newVal: ' + toJson(value) + '; oldVal: ' + toJson(last); + watchLog[logIdx].push(logMsg); + } + } else if (watch === lastDirtyWatch) { + // If the most recently dirty watcher is now clean, short circuit since the remaining watchers + // have already been tested. + dirty = false; + break traverseScopesLoop; } } } catch (e) { @@ -604,13 +619,16 @@ function $RootScopeProvider(){ // Insanity Warning: scope depth-first traversal // yes, this code is a bit crazy, but it works and we have tests to prove it! // this piece should be kept in sync with the traversal in $broadcast - if (!(next = (current.$$childHead || (current !== target && current.$$nextSibling)))) { + if (!(next = (current.$$childHead || + (current !== target && current.$$nextSibling)))) { while(current !== target && !(next = current.$$nextSibling)) { current = current.$parent; } } } while ((current = next)); + // `break traverseScopesLoop;` takes us to here + if(dirty && !(ttl--)) { clearPhase(); throw $rootScopeMinErr('infdig', @@ -618,6 +636,7 @@ function $RootScopeProvider(){ 'Watchers fired in the last 5 iterations: {1}', TTL, toJson(watchLog)); } + } while (dirty || asyncQueue.length); clearPhase(); diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 287b535634da..cc6727c21cdc 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -577,6 +577,82 @@ describe('Scope', function() { }); }); }); + + + describe('optimizations', function() { + + function setupWatches(scope, log) { + scope.$watch(function() { log('w1'); return scope.w1; }, log.fn('w1action')); + scope.$watch(function() { log('w2'); return scope.w2; }, log.fn('w2action')); + scope.$watch(function() { log('w3'); return scope.w3; }, log.fn('w3action')); + scope.$digest(); + log.reset(); + } + + + it('should check watches only once during an empty digest', inject(function(log, $rootScope) { + setupWatches($rootScope, log); + $rootScope.$digest(); + expect(log).toEqual(['w1', 'w2', 'w3']); + })); + + + it('should quit digest early after we check the last watch that was previously dirty', + inject(function(log, $rootScope) { + setupWatches($rootScope, log); + $rootScope.w1 = 'x'; + $rootScope.$digest(); + expect(log).toEqual(['w1', 'w1action', 'w2', 'w3', 'w1']); + })); + + + it('should not quit digest early if a new watch was added from an existing watch action', + inject(function(log, $rootScope) { + setupWatches($rootScope, log); + $rootScope.$watch(log.fn('w4'), function() { + log('w4action'); + $rootScope.$watch(log.fn('w5'), log.fn('w5action')); + }); + $rootScope.$digest(); + expect(log).toEqual(['w1', 'w2', 'w3', 'w4', 'w4action', + 'w1', 'w2', 'w3', 'w4', 'w5', 'w5action', + 'w1', 'w2', 'w3', 'w4', 'w5']); + })); + + + it('should not quit digest early if an evalAsync task was scheduled from a watch action', + inject(function(log, $rootScope) { + setupWatches($rootScope, log); + $rootScope.$watch(log.fn('w4'), function() { + log('w4action'); + $rootScope.$evalAsync(function() { + log('evalAsync') + }); + }); + $rootScope.$digest(); + expect(log).toEqual(['w1', 'w2', 'w3', 'w4', 'w4action', 'evalAsync', + 'w1', 'w2', 'w3', 'w4']); + })); + + + it('should quit digest early but not too early when various watches fire', inject(function(log, $rootScope) { + setupWatches($rootScope, log); + $rootScope.$watch(function() { log('w4'); return $rootScope.w4; }, function(newVal) { + log('w4action'); + $rootScope.w2 = newVal; + }); + + $rootScope.$digest(); + log.reset(); + + $rootScope.w1 = 'x'; + $rootScope.w4 = 'x'; + $rootScope.$digest(); + expect(log).toEqual(['w1', 'w1action', 'w2', 'w3', 'w4', 'w4action', + 'w1', 'w2', 'w2action', 'w3', 'w4', + 'w1', 'w2']); + })); + }); });