perf(change_detection): removed the currentProto property

modules/angular2/src/change_detection/abstract_change_detector.ts

@@ -1,11 +1,16 @@
-import {isPresent, BaseException} from 'angular2/src/facade/lang';
+import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang';
 import {List, ListWrapper} from 'angular2/src/facade/collection';
 import {ChangeDetectionUtil} from './change_detection_util';
 import {ChangeDetectorRef} from './change_detector_ref';
 import {DirectiveRecord} from './directive_record';
 import {ChangeDetector, ChangeDispatcher} from './interfaces';
-import {ChangeDetectionError} from './exceptions';
+import {
+  ChangeDetectionError,
+  ExpressionChangedAfterItHasBeenCheckedException,
+  DehydratedException
+} from './exceptions';
 import {ProtoRecord} from './proto_record';
+import {BindingRecord} from './binding_record';
 import {Locals} from './parser/locals';
 import {Pipes} from './pipes/pipes';
 import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
@@ -26,12 +31,12 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
   // change detection will fail.
   alreadyChecked: any = false;
   context: T;
-  currentProto: ProtoRecord = null;
   directiveRecords: List<DirectiveRecord>;
   dispatcher: ChangeDispatcher;
   locals: Locals = null;
   mode: string = null;
   pipes: Pipes = null;
+  firstProtoInCurrentBinding: number;
   protos: List<ProtoRecord>;
 
   constructor(public id: string, dispatcher: ChangeDispatcher, protos: List<ProtoRecord>,
@@ -80,24 +85,25 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
   // implementation of `detectChangesInRecordsInternal` which does the work of detecting changes
   // and which this method will call.
   // This method expects that `detectChangesInRecordsInternal` will set the property
-  // `this.currentProto` to whichever [ProtoRecord] is currently being processed. This is to
+  // `this.firstProtoInCurrentBinding` to the selfIndex of the first proto record. This is to
   // facilitate error reporting.
   detectChangesInRecords(throwOnChange: boolean): void {
     if (!this.hydrated()) {
-      ChangeDetectionUtil.throwDehydrated();
+      this.throwDehydratedError();
     }
     try {
       this.detectChangesInRecordsInternal(throwOnChange);
     } catch (e) {
-      this.throwError(this.currentProto, e, e.stack);
+      this._throwError(e, e.stack);
     }
   }
 
   // Subclasses should override this method to perform any work necessary to detect and report
   // changes. For example, changes should be reported via `ChangeDetectionUtil.addChange`, lifecycle
   // methods should be called, etc.
-  // This implementation should also set `this.currentProto` to whichever [ProtoRecord] is
+  // This implementation should also set `this.firstProtoInCurrentBinding` to the selfIndex of the
-  // currently being processed to facilitate error reporting. See {@link #detectChangesInRecords}.
+  // first proto record
+  // to facilitate error reporting. See {@link #detectChangesInRecords}.
   detectChangesInRecordsInternal(throwOnChange: boolean): void {}
 
   // This method is not intended to be overridden. Subclasses should instead provide an
@@ -155,11 +161,40 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
     }
   }
 
-  throwError(proto: ProtoRecord, exception: any, stack: any): void {
+  protected notifyDispatcher(value: any): void {
+    this.dispatcher.notifyOnBinding(this._currentBinding(), value);
+  }
+
+  protected addChange(changes: StringMap<string, any>, oldValue: any,
+                      newValue: any): StringMap<string, any> {
+    if (isBlank(changes)) {
+      changes = {};
+    }
+    changes[this._currentBinding().propertyName] =
+        ChangeDetectionUtil.simpleChange(oldValue, newValue);
+    return changes;
+  }
+
+  private _throwError(exception: any, stack: any): void {
+    var proto = this._currentBindingProto();
     var c = this.dispatcher.getDebugContext(proto.bindingRecord.elementIndex, proto.directiveIndex);
     var context = isPresent(c) ? new _Context(c.element, c.componentElement, c.directive, c.context,
                                               c.locals, c.injector, proto.expressionAsString) :
                                  null;
     throw new ChangeDetectionError(proto, exception, stack, context);
   }
+
+  protected throwOnChangeError(oldValue: any, newValue: any): void {
+    var change = ChangeDetectionUtil.simpleChange(oldValue, newValue);
+    throw new ExpressionChangedAfterItHasBeenCheckedException(this._currentBindingProto(), change,
+                                                              null);
+  }
+
+  protected throwDehydratedError(): void { throw new DehydratedException(); }
+
+  private _currentBinding(): BindingRecord { return this._currentBindingProto().bindingRecord; }
+
+  private _currentBindingProto(): ProtoRecord {
+    return ChangeDetectionUtil.protoByIndex(this.protos, this.firstProtoInCurrentBinding);
+  }
 }

modules/angular2/src/change_detection/change_detection_jit_generator.ts

@@ -46,8 +46,6 @@ export class ChangeDetectorJITGenerator {
       ${this._typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype);
 
       ${this._typeName}.prototype.detectChangesInRecordsInternal = function(throwOnChange) {
-        ${this._names.getCurrentProtoName()} = null;
-
         ${this._names.genInitLocals()}
         var ${IS_CHANGED_LOCAL} = false;
         var ${CHANGES_LOCAL} = null;
@@ -149,7 +147,11 @@ export class ChangeDetectorJITGenerator {
     } else {
       rec = this._genReferenceCheck(r);
     }
-    return `${rec}${this._maybeGenLastInDirective(r)}`;
+    return `
+      ${this._maybeFirstInBinding(r)}
+      ${rec}
+      ${this._maybeGenLastInDirective(r)}
+    `;
   }
 
   _genDirectiveLifecycle(r: ProtoRecord): string {
@@ -173,12 +175,9 @@ export class ChangeDetectorJITGenerator {
 
     var pipe = this._names.getPipeName(r.selfIndex);
     var cdRef = "this.ref";
-
-    var protoIndex = r.selfIndex - 1;
     var pipeType = r.name;
 
     var read = `
-      ${this._names.getCurrentProtoName()} = ${this._names.getProtosName()}[${protoIndex}];
       if (${pipe} === ${UTIL}.uninitialized) {
         ${pipe} = ${this._names.getPipesAccessorName()}.get('${pipeType}', ${context}, ${cdRef});
       } else if (!${pipe}.supports(${context})) {
@@ -204,11 +203,7 @@ export class ChangeDetectorJITGenerator {
   _genReferenceCheck(r: ProtoRecord): string {
     var oldValue = this._names.getFieldName(r.selfIndex);
     var newValue = this._names.getLocalName(r.selfIndex);
-
-    var protoIndex = r.selfIndex - 1;
-
     var read = `
-      ${this._names.getCurrentProtoName()} = ${this._names.getProtosName()}[${protoIndex}];
       ${this._genUpdateCurrentValue(r)}
     `;
 
@@ -331,8 +326,7 @@ export class ChangeDetectorJITGenerator {
     } else {
       return `
         ${this._genThrowOnChangeCheck(oldValue, newValue)}
-        ${this._names.getDispatcherName()}.notifyOnBinding(
+        this.notifyDispatcher(${newValue});
-            ${this._names.getCurrentProtoName()}.bindingRecord, ${newValue});
       `;
     }
   }
@@ -341,7 +335,7 @@ export class ChangeDetectorJITGenerator {
     if (this.generateCheckNoChanges) {
       return `
         if(throwOnChange) {
-          ${UTIL}.throwOnChange(${this._names.getCurrentProtoName()}, ${UTIL}.simpleChange(${oldValue}, ${newValue}));
+          this.throwOnChangeError(${oldValue}, ${newValue});
         }
         `;
     } else {
@@ -361,11 +355,13 @@ export class ChangeDetectorJITGenerator {
     var newValue = this._names.getLocalName(r.selfIndex);
     var oldValue = this._names.getFieldName(r.selfIndex);
     if (!r.bindingRecord.callOnChange()) return "";
-    return `
+    return `${CHANGES_LOCAL} = this.addChange(${CHANGES_LOCAL}, ${oldValue}, ${newValue});`;
-      ${CHANGES_LOCAL} = ${UTIL}.addChange(
+  }
-          ${CHANGES_LOCAL}, ${this._names.getCurrentProtoName()}.bindingRecord.propertyName,
+
-          ${UTIL}.simpleChange(${oldValue}, ${newValue}));
+  _maybeFirstInBinding(r: ProtoRecord): string {
-    `;
+    var prev = ChangeDetectionUtil.protoByIndex(this.records, r.selfIndex - 1);
+    var firstInBindng = isBlank(prev) || prev.bindingRecord !== r.bindingRecord;
+    return firstInBindng ? `${this._names.getFirstProtoInCurrentBinding()} = ${r.selfIndex};` : '';
   }
 
   _maybeGenLastInDirective(r: ProtoRecord): string {

modules/angular2/src/change_detection/change_detection_util.ts

@@ -1,7 +1,6 @@
 import {CONST_EXPR, isPresent, isBlank, BaseException, Type} from 'angular2/src/facade/lang';
 import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
 import {ProtoRecord} from './proto_record';
-import {DehydratedException, ExpressionChangedAfterItHasBeenCheckedException} from './exceptions';
 import {WrappedValue} from './pipes/pipe';
 import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
 
@@ -126,12 +125,6 @@ export class ChangeDetectionUtil {
     }
   }
 
-  static throwOnChange(proto: ProtoRecord, change) {
-    throw new ExpressionChangedAfterItHasBeenCheckedException(proto, change, null);
-  }
-
-  static throwDehydrated() { throw new DehydratedException(); }
-
   static changeDetectionMode(strategy: string): string {
     return strategy == ON_PUSH ? CHECK_ONCE : CHECK_ALWAYS;
   }
@@ -140,15 +133,13 @@ export class ChangeDetectionUtil {
     return _simpleChange(previousValue, currentValue);
   }
 
-  static addChange(changes, propertyName: string, change): Map<any, any> {
-    if (isBlank(changes)) {
-      changes = {};
-    }
-    changes[propertyName] = change;
-    return changes;
-  }
-
   static isValueBlank(value: any): boolean { return isBlank(value); }
 
   static s(value: any): string { return isPresent(value) ? `${value}` : ''; }
+
+  static protoByIndex(protos: ProtoRecord[], selfIndex: number): ProtoRecord {
+    return selfIndex < 1 ?
+               null :
+               protos[selfIndex - 1];  // self index is shifted by one because of context
+  }
 }

modules/angular2/src/change_detection/codegen_name_util.ts

@@ -9,7 +9,7 @@ import {ProtoRecord} from './proto_record';
 // detection will fail.
 const _ALREADY_CHECKED_ACCESSOR = "alreadyChecked";
 const _CONTEXT_ACCESSOR = "context";
-const _CURRENT_PROTO = "currentProto";
+const _FIRST_PROTO_IN_CURRENT_BINDING = "firstProtoInCurrentBinding";
 const _DIRECTIVES_ACCESSOR = "directiveRecords";
 const _DISPATCHER_ACCESSOR = "dispatcher";
 const _LOCALS_ACCESSOR = "locals";
@@ -67,7 +67,9 @@ export class CodegenNameUtil {
 
   getModeName(): string { return this._addFieldPrefix(_MODE_ACCESSOR); }
 
-  getCurrentProtoName(): string { return this._addFieldPrefix(_CURRENT_PROTO); }
+  getFirstProtoInCurrentBinding(): string {
+    return this._addFieldPrefix(_FIRST_PROTO_IN_CURRENT_BINDING);
+  }
 
   getLocalName(idx: int): string { return `l_${this._sanitizedNames[idx]}`; }
 

modules/angular2/src/change_detection/dynamic_change_detector.ts

@@ -55,12 +55,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
 
   checkNoChanges(): void { this.runDetectChanges(true); }
 
-  // TODO(vsavkin): #3366. Update to work with [AbstractChangeDetector#detectChangesInRecords]
+  detectChangesInRecordsInternal(throwOnChange: boolean) {
-  detectChangesInRecords(throwOnChange: boolean) {
+    var protos = this.protos;
-    if (!this.hydrated()) {
-      ChangeDetectionUtil.throwDehydrated();
-    }
-    var protos: List<ProtoRecord> = this.protos;
 
     var changes = null;
     var isChanged = false;
@@ -69,6 +65,10 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
       var bindingRecord = proto.bindingRecord;
       var directiveRecord = bindingRecord.directiveRecord;
 
+      if (this._firstInBinding(proto)) {
+        this.firstProtoInCurrentBinding = proto.selfIndex;
+      }
+
       if (proto.isLifeCycleRecord()) {
         if (proto.name === "onCheck" && !throwOnChange) {
           this._getDirectiveFor(directiveRecord.directiveIndex).onCheck();
@@ -100,6 +100,11 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
     this.alreadyChecked = true;
   }
 
+  _firstInBinding(r: ProtoRecord): boolean {
+    var prev = ChangeDetectionUtil.protoByIndex(this.protos, r.selfIndex - 1);
+    return isBlank(prev) || prev.bindingRecord !== r.bindingRecord;
+  }
+
   callOnAllChangesDone() {
     this.dispatcher.notifyOnAllChangesDone();
     var dirs = this.directiveRecords;
@@ -122,7 +127,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
 
   _addChange(bindingRecord: BindingRecord, change, changes) {
     if (bindingRecord.callOnChange()) {
-      return ChangeDetectionUtil.addChange(changes, bindingRecord.propertyName, change);
+      return super.addChange(changes, change.previousValue, change.currentValue);
     } else {
       return changes;
     }
@@ -133,15 +138,11 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
   _getDetectorFor(directiveIndex) { return this.directives.getDetectorFor(directiveIndex); }
 
   _check(proto: ProtoRecord, throwOnChange: boolean): SimpleChange {
-    try {
     if (proto.isPipeRecord()) {
       return this._pipeCheck(proto, throwOnChange);
     } else {
       return this._referenceCheck(proto, throwOnChange);
     }
-    } catch (e) {
-      this.throwError(proto, e, e.stack);
-    }
   }
 
   _referenceCheck(proto: ProtoRecord, throwOnChange: boolean) {
@@ -156,7 +157,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
       if (!isSame(prevValue, currValue)) {
         if (proto.lastInBinding) {
           var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
-          if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
+          if (throwOnChange) this.throwOnChangeError(prevValue, currValue);
 
           this._writeSelf(proto, currValue);
           this._setChanged(proto, true);
@@ -241,7 +242,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
 
         if (proto.lastInBinding) {
           var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
-          if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
+          if (throwOnChange) this.throwOnChangeError(prevValue, currValue);
 
           this._writeSelf(proto, currValue);
           this._setChanged(proto, true);

modules/angular2/src/transform/template_compiler/change_detector_codegen.dart

@@ -109,8 +109,6 @@ class _CodegenState {
         }
 
         void detectChangesInRecordsInternal(throwOnChange) {
-          ${_names.getCurrentProtoName()} = null;
-
           ${_names.genInitLocals()}
           var $_IS_CHANGED_LOCAL = false;
           var $_CHANGES_LOCAL = null;
@@ -225,7 +223,11 @@ class _CodegenState {
     } else {
       rec = _genReferenceCheck(r);
     }
-    return '$rec${_maybeGenLastInDirective(r)}';
+    return '''
+      ${this._maybeFirstInBinding(r)}
+      ${rec}
+      ${this._maybeGenLastInDirective(r)}
+    ''';
   }
 
   String _genDirectiveLifecycle(ProtoRecord r) {
@@ -249,12 +251,9 @@ class _CodegenState {
 
     var pipe = _names.getPipeName(r.selfIndex);
     var cdRef = 'this.ref';
-
-    var protoIndex = r.selfIndex - 1;
     var pipeType = r.name;
 
     var read = '''
-      ${_names.getCurrentProtoName()} = ${_names.getProtosName()}[$protoIndex];
       if ($_IDENTICAL_CHECK_FN($pipe, $_UTIL.uninitialized)) {
         $pipe = ${_names.getPipesAccessorName()}.get('$pipeType', $context, $cdRef);
       } else if (!$pipe.supports($context)) {
@@ -280,11 +279,7 @@ class _CodegenState {
   String _genReferenceCheck(ProtoRecord r) {
     var oldValue = _names.getFieldName(r.selfIndex);
     var newValue = _names.getLocalName(r.selfIndex);
-
-    var protoIndex = r.selfIndex - 1;
-
     var read = '''
-      ${_names.getCurrentProtoName()} = ${_names.getProtosName()}[$protoIndex];
       ${_genUpdateCurrentValue(r)}
     ''';
 
@@ -411,8 +406,7 @@ class _CodegenState {
     } else {
       return '''
       ${_genThrowOnChangeCheck(oldValue, newValue)}
-      ${_names.getDispatcherName()}.notifyOnBinding(
+      this.notifyDispatcher(${newValue});
-          ${_names.getCurrentProtoName()}.bindingRecord, ${newValue});
     ''';
     }
   }
@@ -421,8 +415,7 @@ class _CodegenState {
     if (this._generateCheckNoChanges) {
       return '''
         if(throwOnChange) {
-          $_UTIL.throwOnChange(
+          this.throwOnChangeError(${oldValue}, ${newValue});
-              ${_names.getCurrentProtoName()}, $_UTIL.simpleChange(${oldValue}, ${newValue}));
         }
       ''';
     } else {
@@ -438,16 +431,17 @@ class _CodegenState {
     }
   }
 
+  String _maybeFirstInBinding(ProtoRecord r) {
+    var prev = ChangeDetectionUtil.protoByIndex(_records, r.selfIndex - 1);
+    var firstInBindng = prev == null || prev.bindingRecord != r.bindingRecord;
+    return firstInBindng ? "${_names.getFirstProtoInCurrentBinding()} = ${r.selfIndex};" : '';
+  }
+
   String _genAddToChanges(ProtoRecord r) {
     var newValue = _names.getLocalName(r.selfIndex);
     var oldValue = _names.getFieldName(r.selfIndex);
     if (!r.bindingRecord.callOnChange()) return '';
-    return '''
+    return "$_CHANGES_LOCAL = addChange($_CHANGES_LOCAL, $oldValue, $newValue);";
-      $_CHANGES_LOCAL = $_UTIL.addChange(
-          $_CHANGES_LOCAL,
-          ${_names.getCurrentProtoName()}.bindingRecord.propertyName,
-          $_UTIL.simpleChange($oldValue, $newValue));
-    ''';
   }
 
   String _maybeGenLastInDirective(ProtoRecord r) {

modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart

@@ -34,7 +34,6 @@ class _MyComponent_ChangeDetector0
   }
 
   void detectChangesInRecordsInternal(throwOnChange) {
-    this.currentProto = null;
     var l_context = this.context,
         l_myNum0,
         c_myNum0,
@@ -43,7 +42,7 @@ class _MyComponent_ChangeDetector0
     var isChanged = false;
     var changes = null;
 
-    this.currentProto = this.protos[0];
+    this.firstProtoInCurrentBinding = 1;
     l_myNum0 = l_context.myNum;
     if (_gen.looseNotIdentical(l_myNum0, this.myNum0)) {
       c_myNum0 = true;
@@ -51,18 +50,14 @@ class _MyComponent_ChangeDetector0
       this.myNum0 = l_myNum0;
     }
     if (c_myNum0) {
-      this.currentProto = this.protos[1];
       l_interpolate1 =
           "Salad: " "${l_myNum0 == null ? "" : l_myNum0}" " is awesome";
       if (_gen.looseNotIdentical(l_interpolate1, this.interpolate1)) {
         if (throwOnChange) {
-          _gen.ChangeDetectionUtil.throwOnChange(this.currentProto,
+          this.throwOnChangeError(this.interpolate1, l_interpolate1);
-              _gen.ChangeDetectionUtil.simpleChange(
-                  this.interpolate1, l_interpolate1));
         }
 
-        this.dispatcher.notifyOnBinding(
+        this.notifyDispatcher(l_interpolate1);
-            this.currentProto.bindingRecord, l_interpolate1);
 
         this.interpolate1 = l_interpolate1;
       }

modules/angular2/test/transform/template_compiler/all_tests.dart

@@ -31,10 +31,10 @@ void changeDetectorTests() {
   // TODO(tbosch): This is just a temporary test that makes sure that the dart server and
   // dart browser is in sync. Change this to "not contains notifyBinding"
   // when https://github.com/angular/angular/issues/3019 is solved.
-  it('shouldn always notifyBinding for template variables', () async {
+  it('shouldn always notifyDispatcher for template variables', () async {
     var inputPath = 'template_compiler/ng_for_files/hello.ng_deps.dart';
     var output = await (process(new AssetId('a', inputPath)));
-    expect(output).toContain('notifyOnBinding');
+    expect(output).toContain('notifyDispatcher');
   });
 
   it('should include directives mentioned in directive aliases.', () async {