Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit d070450

Browse files
kseamonIgorMinar
authored andcommitted
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
1 parent 09648e4 commit d070450

File tree

2 files changed

+112
-17
lines changed

2 files changed

+112
-17
lines changed

src/ng/rootScope.js

+36-17
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
function $RootScopeProvider(){
7272
var TTL = 10;
7373
var $rootScopeMinErr = minErr('$rootScope');
74+
var lastDirtyWatch = null;
7475

7576
this.digestTtl = function(value) {
7677
if (arguments.length) {
@@ -325,6 +326,8 @@ function $RootScopeProvider(){
325326
eq: !!objectEquality
326327
};
327328

329+
lastDirtyWatch = null;
330+
328331
// in the case user pass string, we need to compile it, do we really need this ?
329332
if (!isFunction(listener)) {
330333
var listenFn = compileToFn(listener || noop, 'listener');
@@ -553,6 +556,8 @@ function $RootScopeProvider(){
553556

554557
beginPhase('$digest');
555558

559+
lastDirtyWatch = null;
560+
556561
do { // "while dirty" loop
557562
dirty = false;
558563
current = target;
@@ -565,8 +570,10 @@ function $RootScopeProvider(){
565570
clearPhase();
566571
$exceptionHandler(e);
567572
}
573+
lastDirtyWatch = null;
568574
}
569575

576+
traverseScopesLoop:
570577
do { // "traverse the scopes" loop
571578
if ((watchers = current.$$watchers)) {
572579
// process our watches
@@ -576,22 +583,30 @@ function $RootScopeProvider(){
576583
watch = watchers[length];
577584
// Most common watches are on primitives, in which case we can short
578585
// circuit it with === operator, only when === fails do we use .equals
579-
if (watch && (value = watch.get(current)) !== (last = watch.last) &&
580-
!(watch.eq
581-
? equals(value, last)
582-
: (typeof value == 'number' && typeof last == 'number'
583-
&& isNaN(value) && isNaN(last)))) {
584-
dirty = true;
585-
watch.last = watch.eq ? copy(value) : value;
586-
watch.fn(value, ((last === initWatchVal) ? value : last), current);
587-
if (ttl < 5) {
588-
logIdx = 4 - ttl;
589-
if (!watchLog[logIdx]) watchLog[logIdx] = [];
590-
logMsg = (isFunction(watch.exp))
591-
? 'fn: ' + (watch.exp.name || watch.exp.toString())
592-
: watch.exp;
593-
logMsg += '; newVal: ' + toJson(value) + '; oldVal: ' + toJson(last);
594-
watchLog[logIdx].push(logMsg);
586+
if (watch) {
587+
if ((value = watch.get(current)) !== (last = watch.last) &&
588+
!(watch.eq
589+
? equals(value, last)
590+
: (typeof value == 'number' && typeof last == 'number'
591+
&& isNaN(value) && isNaN(last)))) {
592+
dirty = true;
593+
lastDirtyWatch = watch;
594+
watch.last = watch.eq ? copy(value) : value;
595+
watch.fn(value, ((last === initWatchVal) ? value : last), current);
596+
if (ttl < 5) {
597+
logIdx = 4 - ttl;
598+
if (!watchLog[logIdx]) watchLog[logIdx] = [];
599+
logMsg = (isFunction(watch.exp))
600+
? 'fn: ' + (watch.exp.name || watch.exp.toString())
601+
: watch.exp;
602+
logMsg += '; newVal: ' + toJson(value) + '; oldVal: ' + toJson(last);
603+
watchLog[logIdx].push(logMsg);
604+
}
605+
} else if (watch === lastDirtyWatch) {
606+
// If the most recently dirty watcher is now clean, short circuit since the remaining watchers
607+
// have already been tested.
608+
dirty = false;
609+
break traverseScopesLoop;
595610
}
596611
}
597612
} catch (e) {
@@ -604,20 +619,24 @@ function $RootScopeProvider(){
604619
// Insanity Warning: scope depth-first traversal
605620
// yes, this code is a bit crazy, but it works and we have tests to prove it!
606621
// this piece should be kept in sync with the traversal in $broadcast
607-
if (!(next = (current.$$childHead || (current !== target && current.$$nextSibling)))) {
622+
if (!(next = (current.$$childHead ||
623+
(current !== target && current.$$nextSibling)))) {
608624
while(current !== target && !(next = current.$$nextSibling)) {
609625
current = current.$parent;
610626
}
611627
}
612628
} while ((current = next));
613629

630+
// `break traverseScopesLoop;` takes us to here
631+
614632
if(dirty && !(ttl--)) {
615633
clearPhase();
616634
throw $rootScopeMinErr('infdig',
617635
'{0} $digest() iterations reached. Aborting!\n' +
618636
'Watchers fired in the last 5 iterations: {1}',
619637
TTL, toJson(watchLog));
620638
}
639+
621640
} while (dirty || asyncQueue.length);
622641

623642
clearPhase();

test/ng/rootScopeSpec.js

+76
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,82 @@ describe('Scope', function() {
577577
});
578578
});
579579
});
580+
581+
582+
describe('optimizations', function() {
583+
584+
function setupWatches(scope, log) {
585+
scope.$watch(function() { log('w1'); return scope.w1; }, log.fn('w1action'));
586+
scope.$watch(function() { log('w2'); return scope.w2; }, log.fn('w2action'));
587+
scope.$watch(function() { log('w3'); return scope.w3; }, log.fn('w3action'));
588+
scope.$digest();
589+
log.reset();
590+
}
591+
592+
593+
it('should check watches only once during an empty digest', inject(function(log, $rootScope) {
594+
setupWatches($rootScope, log);
595+
$rootScope.$digest();
596+
expect(log).toEqual(['w1', 'w2', 'w3']);
597+
}));
598+
599+
600+
it('should quit digest early after we check the last watch that was previously dirty',
601+
inject(function(log, $rootScope) {
602+
setupWatches($rootScope, log);
603+
$rootScope.w1 = 'x';
604+
$rootScope.$digest();
605+
expect(log).toEqual(['w1', 'w1action', 'w2', 'w3', 'w1']);
606+
}));
607+
608+
609+
it('should not quit digest early if a new watch was added from an existing watch action',
610+
inject(function(log, $rootScope) {
611+
setupWatches($rootScope, log);
612+
$rootScope.$watch(log.fn('w4'), function() {
613+
log('w4action');
614+
$rootScope.$watch(log.fn('w5'), log.fn('w5action'));
615+
});
616+
$rootScope.$digest();
617+
expect(log).toEqual(['w1', 'w2', 'w3', 'w4', 'w4action',
618+
'w1', 'w2', 'w3', 'w4', 'w5', 'w5action',
619+
'w1', 'w2', 'w3', 'w4', 'w5']);
620+
}));
621+
622+
623+
it('should not quit digest early if an evalAsync task was scheduled from a watch action',
624+
inject(function(log, $rootScope) {
625+
setupWatches($rootScope, log);
626+
$rootScope.$watch(log.fn('w4'), function() {
627+
log('w4action');
628+
$rootScope.$evalAsync(function() {
629+
log('evalAsync')
630+
});
631+
});
632+
$rootScope.$digest();
633+
expect(log).toEqual(['w1', 'w2', 'w3', 'w4', 'w4action', 'evalAsync',
634+
'w1', 'w2', 'w3', 'w4']);
635+
}));
636+
637+
638+
it('should quit digest early but not too early when various watches fire', inject(function(log, $rootScope) {
639+
setupWatches($rootScope, log);
640+
$rootScope.$watch(function() { log('w4'); return $rootScope.w4; }, function(newVal) {
641+
log('w4action');
642+
$rootScope.w2 = newVal;
643+
});
644+
645+
$rootScope.$digest();
646+
log.reset();
647+
648+
$rootScope.w1 = 'x';
649+
$rootScope.w4 = 'x';
650+
$rootScope.$digest();
651+
expect(log).toEqual(['w1', 'w1action', 'w2', 'w3', 'w4', 'w4action',
652+
'w1', 'w2', 'w2action', 'w3', 'w4',
653+
'w1', 'w2']);
654+
}));
655+
});
580656
});
581657

582658

0 commit comments

Comments
 (0)