perf(change_detection): do not generate onAllChangesDone when not needed

vsavkin · vsavkin · commit adc27398fddd · 2015-07-31T19:23:00.000Z
diff --git a/modules/angular2/src/change_detection/abstract_change_detector.ts b/modules/angular2/src/change_detection/abstract_change_detector.ts
@@ -135,7 +135,7 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
 
   hydrated(): boolean { return this.context !== null; }
 
-  callOnAllChangesDone(): void {}
+  callOnAllChangesDone(): void { this.dispatcher.notifyOnAllChangesDone(); }
 
   _detectChangesInLightDomChildren(throwOnChange: boolean): void {
     var c = this.lightDomChildren;
diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/change_detection/change_detection_jit_generator.ts
@@ -57,9 +57,7 @@ export class ChangeDetectorJITGenerator {
 
       ${this._genCheckNoChanges()}
 
-      ${this._typeName}.prototype.callOnAllChangesDone = function() {
-        ${this._genCallOnAllChangesDoneBody()}
-      }
+      ${this._maybeGenCallOnAllChangesDone()}
 
       ${this._maybeGenHydrateDirectives()}
 
@@ -117,7 +115,7 @@ export class ChangeDetectorJITGenerator {
     return lines.join('\n');
   }
 
-  _genCallOnAllChangesDoneBody(): string {
+  _maybeGenCallOnAllChangesDone(): string {
     var notifications = [];
     var dirs = this.directiveRecords;
 
@@ -129,13 +127,17 @@ export class ChangeDetectorJITGenerator {
             `${this._names.getDirectiveName(dir.directiveIndex)}.onAllChangesDone();`);
       }
     }
-
-    var directiveNotifications = notifications.join("\n");
-
-    return `
-      ${this._names.getDispatcherName()}.notifyOnAllChangesDone();
-      ${directiveNotifications}
-    `;
+    if (notifications.length > 0) {
+      var directiveNotifications = notifications.join("\n");
+      return `
+        ${this._typeName}.prototype.callOnAllChangesDone = function() {
+          ${ABSTRACT_CHANGE_DETECTOR}.prototype.callOnAllChangesDone.call(this);
+          ${directiveNotifications}
+        }
+      `;
+    } else {
+      return '';
+    }
   }
 
   _genRecord(r: ProtoRecord): string {
diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.ts b/modules/angular2/src/change_detection/dynamic_change_detector.ts
@@ -106,7 +106,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
   }
 
   callOnAllChangesDone() {
-    this.dispatcher.notifyOnAllChangesDone();
+    super.callOnAllChangesDone();
     var dirs = this.directiveRecords;
     for (var i = dirs.length - 1; i >= 0; --i) {
       var dir = dirs[i];
diff --git a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart
@@ -120,9 +120,7 @@ class _CodegenState {
 
         ${_genCheckNoChanges()}
 
-        void callOnAllChangesDone() {
-          ${_getCallOnAllChangesDoneBody()}
-        }
+        ${_maybeGenCallOnAllChangesDone()}
 
         ${_maybeGenHydrateDirectives()}
 
@@ -190,17 +188,23 @@ class _CodegenState {
 
   /// Generates calls to `onAllChangesDone` for all `Directive`s that request
   /// them.
-  String _getCallOnAllChangesDoneBody() {
+  String _maybeGenCallOnAllChangesDone() {
     // NOTE(kegluneq): Order is important!
     var directiveNotifications = _directiveRecords.reversed
         .where((rec) => rec.callOnAllChangesDone)
         .map((rec) =>
-            '${_names.getDirectiveName(rec.directiveIndex)}.onAllChangesDone();')
-        .join('');
-    return '''
-      ${_names.getDispatcherName()}.notifyOnAllChangesDone();
-      ${directiveNotifications}
-    ''';
+            '${_names.getDirectiveName(rec.directiveIndex)}.onAllChangesDone();');
+
+    if (directiveNotifications.isNotEmpty) {
+      return '''
+        void callOnAllChangesDone() {
+          ${_names.getDispatcherName()}.notifyOnAllChangesDone();
+          ${directiveNotifications.join('')}
+        }
+      ''';
+    } else {
+      return '';
+    }
   }
 
   String _genDeclareFields() {
diff --git a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart
@@ -73,10 +73,6 @@ class _MyComponent_ChangeDetector0
     runDetectChanges(true);
   }
 
-  void callOnAllChangesDone() {
-    this.dispatcher.notifyOnAllChangesDone();
-  }
-
   void dehydrateDirectives(destroyPipes) {
     this.myNum0 = this.interpolate1 = _gen.ChangeDetectionUtil.uninitialized;
   }

Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {`
`106`	`106`	`}`
`107`	`107`
`108`	`108`	`callOnAllChangesDone() {`
`109`		`- this.dispatcher.notifyOnAllChangesDone();`
	`109`	`+ super.callOnAllChangesDone();`
`110`	`110`	`var dirs = this.directiveRecords;`
`111`	`111`	`for (var i = dirs.length - 1; i >= 0; --i) {`
`112`	`112`	`var dir = dirs[i];`
Original file line number	Diff line number	Diff line change
`@@ -73,10 +73,6 @@ class _MyComponent_ChangeDetector0`
`73`	`73`	`runDetectChanges(true);`
`74`	`74`	`}`
`75`	`75`
`76`		`- void callOnAllChangesDone() {`
`77`		`- this.dispatcher.notifyOnAllChangesDone();`
`78`		`- }`
`79`		`-`
`80`	`76`	`void dehydrateDirectives(destroyPipes) {`
`81`	`77`	`this.myNum0 = this.interpolate1 = _gen.ChangeDetectionUtil.uninitialized;`
`82`	`78`	`}`