From 28540c804a1b2e4dd3328b3caca1391c696da1ca Mon Sep 17 00:00:00 2001
From: Igor Minar <igor@angularjs.org>
Date: Tue, 29 Jul 2014 10:46:00 -0700
Subject: [PATCH] perf(Scope): remove the need for the extra watch in
 $watchGroup

Instead of using a counter and an extra watch, just schedule the reaction function via $evalAsync.

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 #8396
---
 src/ng/rootScope.js             | 36 +++++++++++++++++----------------
 test/ng/compileSpec.js          | 28 ++++++++++++-------------
 test/ng/directive/ngBindSpec.js |  4 ++--
 test/ng/rootScopeSpec.js        |  6 +++---
 4 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js
index a52baf63..b92f96b4 100644
--- a/src/ng/rootScope.js
+++ b/src/ng/rootScope.js
@@ -393,9 +393,9 @@ function $RootScopeProvider(){
         var oldValues = new Array(watchExpressions.length);
         var newValues = new Array(watchExpressions.length);
         var deregisterFns = [];
-        var changeCount = 0;
         var self = this;
-        var masterUnwatch;
+        var changeReactionScheduled = false;
+        var firstRun = true;
 
         if (watchExpressions.length === 1) {
           // Special case size of one
@@ -407,29 +407,31 @@ function $RootScopeProvider(){
         }
 
         forEach(watchExpressions, function (expr, i) {
-          var unwatch = self.$watch(expr, function watchGroupSubAction(value, oldValue) {
+          var unwatchFn = self.$watch(expr, function watchGroupSubAction(value, oldValue) {
             newValues[i] = value;
             oldValues[i] = oldValue;
-            changeCount++;
-          }, false, function watchGroupDeregNotifier() {
-            arrayRemove(deregisterFns, unwatch);
-            if (!deregisterFns.length) {
-              masterUnwatch();
+            if (!changeReactionScheduled) {
+              changeReactionScheduled = true;
+              self.$evalAsync(watchGroupAction);
             }
           });
-
-          deregisterFns.push(unwatch);
-        }, this);
-
-        masterUnwatch = self.$watch(function watchGroupChangeWatch() {
-          return changeCount;
-        }, function watchGroupChangeAction(value, oldValue) {
-          listener(newValues, (value === oldValue) ? newValues : oldValues, self);
+          deregisterFns.push(unwatchFn);
         });
 
+        function watchGroupAction() {
+          changeReactionScheduled = false;
+
+          if (firstRun) {
+            firstRun = false;
+            listener(newValues, newValues, self);
+          } else {
+            listener(newValues, oldValues, self);
+          }
+        }
+
         return function deregisterWatchGroup() {
           while (deregisterFns.length) {
-            deregisterFns[0]();
+            deregisterFns.shift()();
           }
         };
       },
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
index fb5dc525..d2f1aec7 100755
--- a/test/ng/compileSpec.js
+++ b/test/ng/compileSpec.js
@@ -2893,21 +2893,21 @@ describe('$compile', function() {
 
         inject(function($rootScope) {
           compile('<div other-tpl-dir param1="::foo" param2="bar"></div>');
-          expect(countWatches($rootScope)).toEqual(7); // 5 -> template watch group, 2 -> '='
+          expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> '='
           $rootScope.$digest();
           expect(element.html()).toBe('1:;2:;3:;4:');
-          expect(countWatches($rootScope)).toEqual(7);
+          expect(countWatches($rootScope)).toEqual(6);
 
           $rootScope.foo = 'foo';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:;3:foo;4:');
-          expect(countWatches($rootScope)).toEqual(5);
+          expect(countWatches($rootScope)).toEqual(4);
 
           $rootScope.foo = 'baz';
           $rootScope.bar = 'bar';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
-          expect(countWatches($rootScope)).toEqual(4);
+          expect(countWatches($rootScope)).toEqual(3);
 
           $rootScope.bar = 'baz';
           $rootScope.$digest();
@@ -2927,21 +2927,21 @@ describe('$compile', function() {
 
         inject(function($rootScope) {
           compile('<div other-tpl-dir param1="{{::foo}}" param2="{{bar}}"></div>');
-          expect(countWatches($rootScope)).toEqual(7); // 5 -> template watch group, 2 -> {{ }}
+          expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> {{ }}
           $rootScope.$digest();
           expect(element.html()).toBe('1:;2:;3:;4:');
-          expect(countWatches($rootScope)).toEqual(5); // (- 2) -> bind-once in template
+          expect(countWatches($rootScope)).toEqual(4); // (- 2) -> bind-once in template
 
           $rootScope.foo = 'foo';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:;3:;4:');
-          expect(countWatches($rootScope)).toEqual(4);
+          expect(countWatches($rootScope)).toEqual(3);
 
           $rootScope.foo = 'baz';
           $rootScope.bar = 'bar';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:bar;3:;4:');
-          expect(countWatches($rootScope)).toEqual(4);
+          expect(countWatches($rootScope)).toEqual(3);
 
           $rootScope.bar = 'baz';
           $rootScope.$digest();
@@ -2964,18 +2964,18 @@ describe('$compile', function() {
           compile('<div other-tpl-dir param1="::foo" param2="bar"></div>');
           $rootScope.$digest();
           expect(element.html()).toBe('1:;2:;3:;4:');
-          expect(countWatches($rootScope)).toEqual(7); // 5 -> template watch group, 2 -> '='
+          expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> '='
 
           $rootScope.foo = 'foo';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:;3:foo;4:');
-          expect(countWatches($rootScope)).toEqual(5);
+          expect(countWatches($rootScope)).toEqual(4);
 
           $rootScope.foo = 'baz';
           $rootScope.bar = 'bar';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
-          expect(countWatches($rootScope)).toEqual(4);
+          expect(countWatches($rootScope)).toEqual(3);
 
           $rootScope.bar = 'baz';
           $rootScope.$digest();
@@ -2998,18 +2998,18 @@ describe('$compile', function() {
           compile('<div other-tpl-dir param1="{{::foo}}" param2="{{bar}}"></div>');
           $rootScope.$digest();
           expect(element.html()).toBe('1:;2:;3:;4:');
-          expect(countWatches($rootScope)).toEqual(5); // (5 - 2) -> template watch group, 2 -> {{ }}
+          expect(countWatches($rootScope)).toEqual(4); // (4 - 2) -> template watch group, 2 -> {{ }}
 
           $rootScope.foo = 'foo';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:;3:;4:');
-          expect(countWatches($rootScope)).toEqual(4);
+          expect(countWatches($rootScope)).toEqual(3);
 
           $rootScope.foo = 'baz';
           $rootScope.bar = 'bar';
           $rootScope.$digest();
           expect(element.html()).toBe('1:foo;2:bar;3:;4:');
-          expect(countWatches($rootScope)).toEqual(4);
+          expect(countWatches($rootScope)).toEqual(3);
 
           $rootScope.bar = 'baz';
           $rootScope.$digest();
diff --git a/test/ng/directive/ngBindSpec.js b/test/ng/directive/ngBindSpec.js
index 166fdb70..704932b0 100644
--- a/test/ng/directive/ngBindSpec.js
+++ b/test/ng/directive/ngBindSpec.js
@@ -99,11 +99,11 @@ describe('ngBind*', function() {
     it('should one-time bind the expressions that start with ::', inject(function($rootScope, $compile) {
       element = $compile('<div ng-bind-template="{{::hello}} {{::name}}!"></div>')($rootScope);
       $rootScope.name = 'Misko';
-      expect($rootScope.$$watchers.length).toEqual(3);
+      expect($rootScope.$$watchers.length).toEqual(2);
       $rootScope.$digest();
       expect(element.hasClass('ng-binding')).toEqual(true);
       expect(element.text()).toEqual(' Misko!');
-      expect($rootScope.$$watchers.length).toEqual(2);
+      expect($rootScope.$$watchers.length).toEqual(1);
       $rootScope.hello = 'Hello';
       $rootScope.name = 'Lucas';
       $rootScope.$digest();
diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js
index 1c26bed7..a6aa91ea 100644
--- a/test/ng/rootScopeSpec.js
+++ b/test/ng/rootScopeSpec.js
@@ -149,14 +149,14 @@ describe('Scope', function() {
 
     it('should clean up stable watches from $watchGroup', inject(function($rootScope) {
       $rootScope.$watchGroup(['::foo', '::bar'], function() {});
-      expect($rootScope.$$watchers.length).toEqual(3);
+      expect($rootScope.$$watchers.length).toEqual(2);
 
       $rootScope.$digest();
-      expect($rootScope.$$watchers.length).toEqual(3);
+      expect($rootScope.$$watchers.length).toEqual(2);
 
       $rootScope.foo = 'foo';
       $rootScope.$digest();
-      expect($rootScope.$$watchers.length).toEqual(2);
+      expect($rootScope.$$watchers.length).toEqual(1);
 
       $rootScope.bar = 'bar';
       $rootScope.$digest();