From 929e564bac20bcc1a40de6906fbd21d5dd8d0e9f Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 26 Nov 2013 10:57:29 -0800 Subject: [PATCH] feat(scope): early exit of digest loop At any given time there are many watchers in the system. When watcher fires it can change the model. This change may now cause an earlier watch to be eligible for firing. For this reason the watches need to be re-checked several times, until no watchers fire. Currently the system loops until there is a clean run through the loop. This means that every watch needs to run at least twice. A better way is to remember the last fired watch, and then exit the loop early when the the last watch has been reached again. --- lib/core/scope.dart | 22 +++++++++++----- test/core/scope_spec.dart | 55 +++++++++++++++++++++++++++++++-------- 2 files changed, 59 insertions(+), 18 deletions(-) diff --git a/lib/core/scope.dart b/lib/core/scope.dart index 798413b32..fcb588028 100644 --- a/lib/core/scope.dart +++ b/lib/core/scope.dart @@ -399,9 +399,11 @@ class Scope implements Map { } $digest() { - var innerAsyncQueue = _innerAsyncQueue, - length, - dirty, _ttlLeft = _ttl; + var innerAsyncQueue = _innerAsyncQueue; + num length; + _Watch lastDirtyWatch = null; + _Watch lastLoopLastDirtyWatch; + num _ttlLeft = _ttl; List> watchLog = []; List<_Watch> watchers; _Watch watch; @@ -412,7 +414,8 @@ class Scope implements Map { var watcherCount; var scopeCount; do { // "while dirty" loop - dirty = false; + lastLoopLastDirtyWatch = lastDirtyWatch; + lastDirtyWatch = null; current = target; //asyncQueue = current._asyncQueue; //dump('aQ: ${asyncQueue.length}'); @@ -433,6 +436,7 @@ class Scope implements Map { watcherCount = 0; scopeCount = 0; assert((timerId = _perf.startTimer('ng.dirty_check', _ttl-_ttlLeft)) != false); + digestLoop: do { // "traverse the scopes" loop scopeCount++; if ((watchers = current._watchers) != null) { @@ -442,10 +446,14 @@ class Scope implements Map { while (length-- > 0) { try { watch = watchers[length]; + if (identical(lastLoopLastDirtyWatch, watch)) { + break digestLoop; + } var value = watch.get(current); var last = watch.last; if (!_identical(value, last)) { - dirty = true; + lastDirtyWatch = watch; + lastLoopLastDirtyWatch = null; watch.last = value; var fireTimer; assert((fireTimer = _perf.startTimer('ng.fire', watch.exp)) != false); @@ -483,11 +491,11 @@ class Scope implements Map { } while ((current = next) != null); assert(_perf.stopTimer(timerId) != false); - if(dirty && (_ttlLeft--) == 0) { + if(lastDirtyWatch != null && (_ttlLeft--) == 0) { throw '$_ttl \$digest() iterations reached. Aborting!\n' + 'Watchers fired in the last 5 iterations: ${_toJson(watchLog)}'; } - } while (dirty || innerAsyncQueue.length > 0); + } while (lastDirtyWatch != null || innerAsyncQueue.length > 0); _perf.counters['ng.scope.watchers'] = watcherCount; _perf.counters['ng.scopes'] = scopeCount; diff --git a/test/core/scope_spec.dart b/test/core/scope_spec.dart index 473c3a1ee..a5664a8e5 100644 --- a/test/core/scope_spec.dart +++ b/test/core/scope_spec.dart @@ -212,7 +212,7 @@ main() { // init $rootScope.$digest(); - expect(log.join('')).toEqual('rabrab'); + expect(log.join('')).toEqual('rabra'); log.removeWhere((e) => true); childA.$digest(); @@ -246,7 +246,7 @@ main() { $rootScope.$watch((a) { log += 'a'; }); child.$watch((a) { log += 'b'; }); $rootScope.$digest(); - expect(log).toEqual('abab'); + expect(log).toEqual('aba'); })); @@ -364,14 +364,14 @@ main() { eagerScope.$watch(() {log += 'eager;';}); rootScope.$digest(); - expect(log).toEqual('lazy;eager;eager;'); + expect(log).toEqual('lazy;eager;'); rootScope.$digest(); - expect(log).toEqual('lazy;eager;eager;eager;'); + expect(log).toEqual('lazy;eager;eager;'); lazyScope.$dirty(); rootScope.$digest(); - expect(log).toEqual('lazy;eager;eager;eager;lazy;eager;'); + expect(log).toEqual('lazy;eager;eager;lazy;eager;'); }); }); @@ -388,17 +388,17 @@ main() { childScope.$watch(() {log += 'digest;';}); rootScope.$digest(); - expect(log).toEqual('digest;digest;'); + expect(log).toEqual('digest;'); childScope.$disabled = true; expect(childScope.$disabled).toEqual(true); rootScope.$digest(); - expect(log).toEqual('digest;digest;'); + expect(log).toEqual('digest;'); childScope.$disabled = false; expect(childScope.$disabled).toEqual(false); rootScope.$digest(); - expect(log).toEqual('digest;digest;digest;'); + expect(log).toEqual('digest;digest;'); }); }); }); @@ -592,7 +592,7 @@ main() { $rootScope.$evalAsync(() => $rootScope.log += 'eval;', outsideDigest: true); $rootScope.$watch(() { $rootScope.log += 'digest;'; }); $rootScope.$digest(); - expect($rootScope.log).toEqual('digest;digest;eval;'); + expect($rootScope.log).toEqual('digest;eval;'); })); it(r'should allow running after digest in issolate scope', inject((Scope $rootScope) { @@ -601,7 +601,7 @@ main() { isolateScope.$evalAsync(() => isolateScope.log += 'eval;', outsideDigest: true); isolateScope.$watch(() { isolateScope.log += 'digest;'; }); isolateScope.$digest(); - expect(isolateScope.log).toEqual('digest;digest;eval;'); + expect(isolateScope.log).toEqual('digest;eval;'); })); }); @@ -1085,7 +1085,6 @@ main() { describe('perf', () { - describe('counters', () { it('should expose scope count', inject((Profiler perf, Scope scope) { @@ -1147,7 +1146,41 @@ main() { expect(perf.counters['ng.scope.watchers']).toEqual(0); })); }); + }); + + + describe('optimizations', () { + var scope; + var log; + beforeEach(inject((Scope _scope, Logger _log) { + scope = _scope; + log = _log; + scope['a'] = 1; + scope['b'] = 2; + scope['c'] = 3; + scope.$watch(() {log('a'); return scope['a'];}, (value) => log('fire:a')); + scope.$watch(() {log('b'); return scope['b'];}, (value) => log('fire:b')); + scope.$watch(() {log('c'); return scope['c'];}, (value) {log('fire:c'); scope['b']++; }); + scope.$digest(); + log.clear(); + })); + + it('should loop once on no dirty', () { + scope.$digest(); + expect(log.result()).toEqual('a; b; c'); + }); + it('should exit early on second loop', () { + scope['b']++; + scope.$digest(); + expect(log.result()).toEqual('a; b; fire:b; c; a'); + }); + + it('should continue checking if second loop dirty', () { + scope['c']++; + scope.$digest(); + expect(log.result()).toEqual('a; b; c; fire:c; a; b; fire:b; c; a'); + }); }); }); }