From adc27398fddd29fe6eab1c417a61c1cb5c7c588b Mon Sep 17 00:00:00 2001 From: vsavkin Date: Fri, 31 Jul 2015 10:35:42 -0700 Subject: [PATCH] perf(change_detection): do not generate onAllChangesDone when not needed --- .../abstract_change_detector.ts | 2 +- .../change_detection_jit_generator.ts | 24 ++++++++++--------- .../dynamic_change_detector.ts | 2 +- .../change_detector_codegen.dart | 24 +++++++++++-------- .../expected/bar.ng_deps.dart | 4 ---- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/modules/angular2/src/change_detection/abstract_change_detector.ts b/modules/angular2/src/change_detection/abstract_change_detector.ts index 644cafd8d7..ed854c9885 100644 --- 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 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 index 3d74321ecc..0baa8e0467 100644 --- 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 index dfbd2a7b0f..21eaa4b141 100644 --- 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 { } 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 index eee47ad8f3..b7adb068bd 100644 --- 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 index a6419c4535..8aa611ed7f 100644 --- 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; }