Skip to content

Commit f82fd2c

Browse files
IgorMinarIgorMinar
committed
perf(Scope): use remove the need for the extra watch in $watchGroup
Instead of using a counter and an extra watch, just schedule the reaction function via . This gives us the same/similar ordering and coalecsing of updates as counter without the extra overhead. Also the code is easier to read. Since interpolation uses watchGroup, this change additionally improves performance of interpolation. In large table benchmark digest cost went down by 15-20% for interpolation. Closes angular#8396
1 parent ca0f59e commit f82fd2c

File tree

4 files changed

+37
-35
lines changed

4 files changed

+37
-35
lines changed

‎src/ng/rootScope.js

+18-16
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,9 @@ function $RootScopeProvider(){
393393
var oldValues = new Array(watchExpressions.length);
394394
var newValues = new Array(watchExpressions.length);
395395
var deregisterFns = [];
396-
var changeCount = 0;
397396
var self = this;
398-
var masterUnwatch;
397+
var changeReactionScheduled = false;
398+
var firstRun = true;
399399

400400
if (watchExpressions.length === 1) {
401401
// Special case size of one
@@ -407,29 +407,31 @@ function $RootScopeProvider(){
407407
}
408408

409409
forEach(watchExpressions, function (expr, i) {
410-
var unwatch = self.$watch(expr, function watchGroupSubAction(value, oldValue) {
410+
var unwatchFn = self.$watch(expr, function watchGroupSubAction(value, oldValue) {
411411
newValues[i] = value;
412412
oldValues[i] = oldValue;
413-
changeCount++;
414-
}, false, function watchGroupDeregNotifier() {
415-
arrayRemove(deregisterFns, unwatch);
416-
if (!deregisterFns.length) {
417-
masterUnwatch();
413+
if (!changeReactionScheduled) {
414+
changeReactionScheduled = true;
415+
self.$evalAsync(watchGroupAction);
418416
}
419417
});
418+
deregisterFns.push(unwatchFn);
419+
});
420420

421-
deregisterFns.push(unwatch);
422-
}, this);
421+
function watchGroupAction() {
422+
changeReactionScheduled = false;
423423

424-
masterUnwatch = self.$watch(function watchGroupChangeWatch() {
425-
return changeCount;
426-
}, function watchGroupChangeAction(value, oldValue) {
427-
listener(newValues, (value === oldValue) ? newValues : oldValues, self);
428-
});
424+
if (firstRun) {
425+
firstRun = false;
426+
listener(newValues, newValues, self);
427+
} else {
428+
listener(newValues, oldValues, self);
429+
}
430+
}
429431

430432
return function deregisterWatchGroup() {
431433
while (deregisterFns.length) {
432-
deregisterFns[0]();
434+
deregisterFns.pop()();
433435
}
434436
};
435437
},

‎test/ng/compileSpec.js

+14-14
Original file line numberDiff line numberDiff line change
@@ -2893,21 +2893,21 @@ describe('$compile', function() {
28932893

28942894
inject(function($rootScope) {
28952895
compile('<div other-tpl-dir param1="::foo" param2="bar"></div>');
2896-
expect(countWatches($rootScope)).toEqual(7); // 5 -> template watch group, 2 -> '='
2896+
expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> '='
28972897
$rootScope.$digest();
28982898
expect(element.html()).toBe('1:;2:;3:;4:');
2899-
expect(countWatches($rootScope)).toEqual(7);
2899+
expect(countWatches($rootScope)).toEqual(6);
29002900

29012901
$rootScope.foo = 'foo';
29022902
$rootScope.$digest();
29032903
expect(element.html()).toBe('1:foo;2:;3:foo;4:');
2904-
expect(countWatches($rootScope)).toEqual(5);
2904+
expect(countWatches($rootScope)).toEqual(4);
29052905

29062906
$rootScope.foo = 'baz';
29072907
$rootScope.bar = 'bar';
29082908
$rootScope.$digest();
29092909
expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
2910-
expect(countWatches($rootScope)).toEqual(4);
2910+
expect(countWatches($rootScope)).toEqual(3);
29112911

29122912
$rootScope.bar = 'baz';
29132913
$rootScope.$digest();
@@ -2927,21 +2927,21 @@ describe('$compile', function() {
29272927

29282928
inject(function($rootScope) {
29292929
compile('<div other-tpl-dir param1="{{::foo}}" param2="{{bar}}"></div>');
2930-
expect(countWatches($rootScope)).toEqual(7); // 5 -> template watch group, 2 -> {{ }}
2930+
expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> {{ }}
29312931
$rootScope.$digest();
29322932
expect(element.html()).toBe('1:;2:;3:;4:');
2933-
expect(countWatches($rootScope)).toEqual(5); // (- 2) -> bind-once in template
2933+
expect(countWatches($rootScope)).toEqual(4); // (- 2) -> bind-once in template
29342934

29352935
$rootScope.foo = 'foo';
29362936
$rootScope.$digest();
29372937
expect(element.html()).toBe('1:foo;2:;3:;4:');
2938-
expect(countWatches($rootScope)).toEqual(4);
2938+
expect(countWatches($rootScope)).toEqual(3);
29392939

29402940
$rootScope.foo = 'baz';
29412941
$rootScope.bar = 'bar';
29422942
$rootScope.$digest();
29432943
expect(element.html()).toBe('1:foo;2:bar;3:;4:');
2944-
expect(countWatches($rootScope)).toEqual(4);
2944+
expect(countWatches($rootScope)).toEqual(3);
29452945

29462946
$rootScope.bar = 'baz';
29472947
$rootScope.$digest();
@@ -2964,18 +2964,18 @@ describe('$compile', function() {
29642964
compile('<div other-tpl-dir param1="::foo" param2="bar"></div>');
29652965
$rootScope.$digest();
29662966
expect(element.html()).toBe('1:;2:;3:;4:');
2967-
expect(countWatches($rootScope)).toEqual(7); // 5 -> template watch group, 2 -> '='
2967+
expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> '='
29682968

29692969
$rootScope.foo = 'foo';
29702970
$rootScope.$digest();
29712971
expect(element.html()).toBe('1:foo;2:;3:foo;4:');
2972-
expect(countWatches($rootScope)).toEqual(5);
2972+
expect(countWatches($rootScope)).toEqual(4);
29732973

29742974
$rootScope.foo = 'baz';
29752975
$rootScope.bar = 'bar';
29762976
$rootScope.$digest();
29772977
expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
2978-
expect(countWatches($rootScope)).toEqual(4);
2978+
expect(countWatches($rootScope)).toEqual(3);
29792979

29802980
$rootScope.bar = 'baz';
29812981
$rootScope.$digest();
@@ -2998,18 +2998,18 @@ describe('$compile', function() {
29982998
compile('<div other-tpl-dir param1="{{::foo}}" param2="{{bar}}"></div>');
29992999
$rootScope.$digest();
30003000
expect(element.html()).toBe('1:;2:;3:;4:');
3001-
expect(countWatches($rootScope)).toEqual(5); // (5 - 2) -> template watch group, 2 -> {{ }}
3001+
expect(countWatches($rootScope)).toEqual(4); // (4 - 2) -> template watch group, 2 -> {{ }}
30023002

30033003
$rootScope.foo = 'foo';
30043004
$rootScope.$digest();
30053005
expect(element.html()).toBe('1:foo;2:;3:;4:');
3006-
expect(countWatches($rootScope)).toEqual(4);
3006+
expect(countWatches($rootScope)).toEqual(3);
30073007

30083008
$rootScope.foo = 'baz';
30093009
$rootScope.bar = 'bar';
30103010
$rootScope.$digest();
30113011
expect(element.html()).toBe('1:foo;2:bar;3:;4:');
3012-
expect(countWatches($rootScope)).toEqual(4);
3012+
expect(countWatches($rootScope)).toEqual(3);
30133013

30143014
$rootScope.bar = 'baz';
30153015
$rootScope.$digest();

‎test/ng/directive/ngBindSpec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ describe('ngBind*', function() {
9999
it('should one-time bind the expressions that start with ::', inject(function($rootScope, $compile) {
100100
element = $compile('<div ng-bind-template="{{::hello}} {{::name}}!"></div>')($rootScope);
101101
$rootScope.name = 'Misko';
102-
expect($rootScope.$$watchers.length).toEqual(3);
102+
expect($rootScope.$$watchers.length).toEqual(2);
103103
$rootScope.$digest();
104104
expect(element.hasClass('ng-binding')).toEqual(true);
105105
expect(element.text()).toEqual(' Misko!');
106-
expect($rootScope.$$watchers.length).toEqual(2);
106+
expect($rootScope.$$watchers.length).toEqual(1);
107107
$rootScope.hello = 'Hello';
108108
$rootScope.name = 'Lucas';
109109
$rootScope.$digest();

‎test/ng/rootScopeSpec.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,14 @@ describe('Scope', function() {
149149

150150
it('should clean up stable watches from $watchGroup', inject(function($rootScope) {
151151
$rootScope.$watchGroup(['::foo', '::bar'], function() {});
152-
expect($rootScope.$$watchers.length).toEqual(3);
152+
expect($rootScope.$$watchers.length).toEqual(2);
153153

154154
$rootScope.$digest();
155-
expect($rootScope.$$watchers.length).toEqual(3);
155+
expect($rootScope.$$watchers.length).toEqual(2);
156156

157157
$rootScope.foo = 'foo';
158158
$rootScope.$digest();
159-
expect($rootScope.$$watchers.length).toEqual(2);
159+
expect($rootScope.$$watchers.length).toEqual(1);
160160

161161
$rootScope.bar = 'bar';
162162
$rootScope.$digest();

0 commit comments

Comments
 (0)