Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Commit

Permalink
feat(scope): early exit of digest loop
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mhevery committed Nov 26, 2013
1 parent 283ea25 commit 929e564
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 18 deletions.
22 changes: 15 additions & 7 deletions lib/core/scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<List<String>> watchLog = [];
List<_Watch> watchers;
_Watch watch;
Expand All @@ -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}');
Expand All @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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;

Expand Down
55 changes: 44 additions & 11 deletions test/core/scope_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ main() {

// init
$rootScope.$digest();
expect(log.join('')).toEqual('rabrab');
expect(log.join('')).toEqual('rabra');

log.removeWhere((e) => true);
childA.$digest();
Expand Down Expand Up @@ -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');
}));


Expand Down Expand Up @@ -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;');
});
});

Expand All @@ -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;');
});
});
});
Expand Down Expand Up @@ -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) {
Expand All @@ -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;');
}));

});
Expand Down Expand Up @@ -1085,7 +1085,6 @@ main() {


describe('perf', () {

describe('counters', () {

it('should expose scope count', inject((Profiler perf, Scope scope) {
Expand Down Expand Up @@ -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');
});
});
});
}

0 comments on commit 929e564

Please sign in to comment.