feat(ChangeDetector): Add support for short-circuiting

2015-10-16 10:55:21 -07:00
parent b91351469f
commit 7e92d2e6b7
10 changed files with 582 additions and 113 deletions
--- a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts
+++ b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts
@ -31,6 +31,7 @@ const CHANGES_LOCAL = "changes";
 export class ChangeDetectorJITGenerator {
  private _logic: CodegenLogicUtil;
  private _names: CodegenNameUtil;
+  private _endOfBlockIdxs: number[];
  private id: string;
  private changeDetectionStrategy: ChangeDetectionStrategy;
  private records: ProtoRecord[];
@ -91,7 +92,7 @@ export class ChangeDetectorJITGenerator {
        var ${IS_CHANGED_LOCAL} = false;
        var ${CHANGES_LOCAL} = null;

-        ${this.records.map((r) => this._genRecord(r)).join("\n")}
+        ${this._genAllRecords(this.records)}
      }

      ${this._maybeGenHandleEventInternal()}
@ -144,10 +145,28 @@ export class ChangeDetectorJITGenerator {

  /** @internal */
  _genEventBinding(eb: EventBinding): string {
-    var recs = eb.records.map(r => this._genEventBindingEval(eb, r)).join("\n");
+    let codes: String[] = [];
+    this._endOfBlockIdxs = [];
+
+    ListWrapper.forEachWithIndex(eb.records, (r, i) => {
+      let code;
+
+      if (r.isConditionalSkipRecord()) {
+        code = this._genConditionalSkip(r, this._names.getEventLocalName(eb, i));
+      } else if (r.isUnconditionalSkipRecord()) {
+        code = this._genUnconditionalSkip(r);
+      } else {
+        code = this._genEventBindingEval(eb, r);
+      }
+
+      code += this._genEndOfSkipBlock(i);
+
+      codes.push(code);
+    });
+
    return `
    if (eventName === "${eb.eventName}" && elIndex === ${eb.elIndex}) {
-      ${recs}
+      ${codes.join("\n")}
    }`;
  }

@ -235,20 +254,65 @@ export class ChangeDetectorJITGenerator {
  }

  /** @internal */
-  _genRecord(r: ProtoRecord): string {
-    var rec;
-    if (r.isLifeCycleRecord()) {
-      rec = this._genDirectiveLifecycle(r);
-    } else if (r.isPipeRecord()) {
-      rec = this._genPipeCheck(r);
-    } else {
-      rec = this._genReferenceCheck(r);
+  _genAllRecords(rs: ProtoRecord[]): string {
+    var codes: String[] = [];
+    this._endOfBlockIdxs = [];
+
+    for (let i = 0; i < rs.length; i++) {
+      let code;
+      let r = rs[i];
+
+      if (r.isLifeCycleRecord()) {
+        code = this._genDirectiveLifecycle(r);
+      } else if (r.isPipeRecord()) {
+        code = this._genPipeCheck(r);
+      } else if (r.isConditionalSkipRecord()) {
+        code = this._genConditionalSkip(r, this._names.getLocalName(r.contextIndex));
+      } else if (r.isUnconditionalSkipRecord()) {
+        code = this._genUnconditionalSkip(r);
+      } else {
+        code = this._genReferenceCheck(r);
+      }
+
+      code = `
+        ${this._maybeFirstInBinding(r)}
+        ${code}
+        ${this._maybeGenLastInDirective(r)}
+        ${this._genEndOfSkipBlock(i)}
+      `;
+
+      codes.push(code);
    }
-    return `
-      ${this._maybeFirstInBinding(r)}
-      ${rec}
-      ${this._maybeGenLastInDirective(r)}
-    `;
+
+    return codes.join("\n");
+  }
+
+  /** @internal */
+  _genConditionalSkip(r: ProtoRecord, condition: string): string {
+    let maybeNegate = r.mode === RecordType.SkipRecordsIf ? '!' : '';
+    this._endOfBlockIdxs.push(r.fixedArgs[0] - 1);
+
+    return `if (${maybeNegate}${condition}) {`;
+  }
+
+  /** @internal */
+  _genUnconditionalSkip(r: ProtoRecord): string {
+    this._endOfBlockIdxs.pop();
+    this._endOfBlockIdxs.push(r.fixedArgs[0] - 1);
+    return `} else {`;
+  }
+
+  /** @internal */
+  _genEndOfSkipBlock(protoIndex: number): string {
+    if (!ListWrapper.isEmpty(this._endOfBlockIdxs)) {
+      let endOfBlock = ListWrapper.last(this._endOfBlockIdxs);
+      if (protoIndex === endOfBlock) {
+        this._endOfBlockIdxs.pop();
+        return '}';
+      }
+    }
+
+    return '';
  }

  /** @internal */
@ -401,8 +465,8 @@ export class ChangeDetectorJITGenerator {
  /** @internal */
  _maybeFirstInBinding(r: ProtoRecord): string {
    var prev = ChangeDetectionUtil.protoByIndex(this.records, r.selfIndex - 1);
-    var firstInBindng = isBlank(prev) || prev.bindingRecord !== r.bindingRecord;
-    return firstInBindng && !r.bindingRecord.isDirectiveLifecycle() ?
+    var firstInBinding = isBlank(prev) || prev.bindingRecord !== r.bindingRecord;
+    return firstInBinding && !r.bindingRecord.isDirectiveLifecycle() ?
               `${this._names.getPropertyBindingIndex()} = ${r.propertyBindingIndex};` :
               '';
  }
--- a/modules/angular2/src/core/change_detection/change_detection_util.ts
+++ b/modules/angular2/src/core/change_detection/change_detection_util.ts
@ -126,8 +126,6 @@ export class ChangeDetectionUtil {
  static operation_greater_then(left, right): any { return left > right; }
  static operation_less_or_equals_then(left, right): any { return left <= right; }
  static operation_greater_or_equals_then(left, right): any { return left >= right; }
-  static operation_logical_and(left, right): any { return left && right; }
-  static operation_logical_or(left, right): any { return left || right; }
  static cond(cond, trueVal, falseVal): any { return cond ? trueVal : falseVal; }

  static mapFn(keys: any[]): any {
--- a/modules/angular2/src/core/change_detection/coalesce.ts
+++ b/modules/angular2/src/core/change_detection/coalesce.ts
@ -1,4 +1,4 @@
-import {isPresent, isBlank, looseIdentical, StringWrapper} from 'angular2/src/core/facade/lang';
+import {isPresent, isBlank, looseIdentical} from 'angular2/src/core/facade/lang';
 import {ListWrapper, Map} from 'angular2/src/core/facade/collection';
 import {RecordType, ProtoRecord} from './proto_record';

@ -13,51 +13,158 @@ import {RecordType, ProtoRecord} from './proto_record';
 *
 * @internal
 */
-export function coalesce(records: ProtoRecord[]): ProtoRecord[] {
-  var res: ProtoRecord[] = [];
-  var indexMap: Map<number, number> = new Map<number, number>();
+export function coalesce(srcRecords: ProtoRecord[]): ProtoRecord[] {
+  let dstRecords = [];
+  let excludedIdxs = [];
+  let indexMap: Map<number, number> = new Map<number, number>();
+  let skipDepth = 0;
+  let skipSources: ProtoRecord[] = ListWrapper.createFixedSize(srcRecords.length);

-  for (var i = 0; i < records.length; ++i) {
-    var r = records[i];
-    var record = _replaceIndices(r, res.length + 1, indexMap);
-    var matchingRecord = _findMatching(record, res);
+  for (let protoIndex = 0; protoIndex < srcRecords.length; protoIndex++) {
+    let skipRecord = skipSources[protoIndex];
+    if (isPresent(skipRecord)) {
+      skipDepth--;
+      skipRecord.fixedArgs[0] = dstRecords.length;
+    }

-    if (isPresent(matchingRecord) && record.lastInBinding) {
-      res.push(_selfRecord(record, matchingRecord.selfIndex, res.length + 1));
-      indexMap.set(r.selfIndex, matchingRecord.selfIndex);
-      matchingRecord.referencedBySelf = true;
-
-    } else if (isPresent(matchingRecord) && !record.lastInBinding) {
-      if (record.argumentToPureFunction) {
-        matchingRecord.argumentToPureFunction = true;
-      }
-
-      indexMap.set(r.selfIndex, matchingRecord.selfIndex);
+    let src = srcRecords[protoIndex];
+    let dst = _cloneAndUpdateIndexes(src, dstRecords, indexMap);

+    if (dst.isSkipRecord()) {
+      dstRecords.push(dst);
+      skipDepth++;
+      skipSources[dst.fixedArgs[0]] = dst;
    } else {
-      res.push(record);
-      indexMap.set(r.selfIndex, record.selfIndex);
+      let record = _mayBeAddRecord(dst, dstRecords, excludedIdxs, skipDepth > 0);
+      indexMap.set(src.selfIndex, record.selfIndex);
    }
  }

-  return res;
+  return _optimizeSkips(dstRecords);
 }

-function _selfRecord(r: ProtoRecord, contextIndex: number, selfIndex: number): ProtoRecord {
+/**
+ * - Conditional skip of 1 record followed by an unconditional skip of N are replaced by  a
+ *   conditional skip of N with the negated condition,
+ * - Skips of 0 records are removed
+ */
+function _optimizeSkips(srcRecords: ProtoRecord[]): ProtoRecord[] {
+  let dstRecords = [];
+  let skipSources = ListWrapper.createFixedSize(srcRecords.length);
+  let indexMap: Map<number, number> = new Map<number, number>();
+
+  for (let protoIndex = 0; protoIndex < srcRecords.length; protoIndex++) {
+    let skipRecord = skipSources[protoIndex];
+    if (isPresent(skipRecord)) {
+      skipRecord.fixedArgs[0] = dstRecords.length;
+    }
+
+    let src = srcRecords[protoIndex];
+
+    if (src.isSkipRecord()) {
+      if (src.isConditionalSkipRecord() && src.fixedArgs[0] === protoIndex + 2 &&
+          protoIndex < srcRecords.length - 1 &&
+          srcRecords[protoIndex + 1].mode === RecordType.SkipRecords) {
+        src.mode = src.mode === RecordType.SkipRecordsIf ? RecordType.SkipRecordsIfNot :
+                                                           RecordType.SkipRecordsIf;
+        src.fixedArgs[0] = srcRecords[protoIndex + 1].fixedArgs[0];
+        protoIndex++;
+      }
+
+      if (src.fixedArgs[0] > protoIndex + 1) {
+        let dst = _cloneAndUpdateIndexes(src, dstRecords, indexMap);
+        dstRecords.push(dst);
+        skipSources[dst.fixedArgs[0]] = dst;
+      }
+
+    } else {
+      let dst = _cloneAndUpdateIndexes(src, dstRecords, indexMap);
+      dstRecords.push(dst);
+      indexMap.set(src.selfIndex, dst.selfIndex);
+    }
+  }
+
+  return dstRecords;
+}
+
+/**
+ * Add a new record or re-use one of the existing records.
+ */
+function _mayBeAddRecord(record: ProtoRecord, dstRecords: ProtoRecord[], excludedIdxs: number[],
+                         excluded: boolean): ProtoRecord {
+  let match = _findFirstMatch(record, dstRecords, excludedIdxs);
+
+  if (isPresent(match)) {
+    if (record.lastInBinding) {
+      dstRecords.push(_createSelfRecord(record, match.selfIndex, dstRecords.length + 1));
+      match.referencedBySelf = true;
+    } else {
+      if (record.argumentToPureFunction) {
+        match.argumentToPureFunction = true;
+      }
+    }
+
+    return match;
+  }
+
+  if (excluded) {
+    excludedIdxs.push(record.selfIndex);
+  }
+
+  dstRecords.push(record);
+  return record;
+}
+
+/**
+ * Returns the first `ProtoRecord` that matches the record.
+ */
+function _findFirstMatch(record: ProtoRecord, dstRecords: ProtoRecord[],
+                         excludedIdxs: number[]): ProtoRecord {
+  return ListWrapper.find(
+      dstRecords,
+      // TODO(vicb): optimize notReusableIndexes.indexOf (sorted array)
+      rr => excludedIdxs.indexOf(rr.selfIndex) == -1 && rr.mode !== RecordType.DirectiveLifecycle &&
+            _haveSameDirIndex(rr, record) && rr.mode === record.mode &&
+            looseIdentical(rr.funcOrValue, record.funcOrValue) &&
+            rr.contextIndex === record.contextIndex && looseIdentical(rr.name, record.name) &&
+            ListWrapper.equals(rr.args, record.args));
+}
+
+/**
+ * Clone the `ProtoRecord` and changes the indexes for the ones in the destination array for:
+ * - the arguments,
+ * - the context,
+ * - self
+ */
+function _cloneAndUpdateIndexes(record: ProtoRecord, dstRecords: ProtoRecord[],
+                                indexMap: Map<number, number>): ProtoRecord {
+  let args = record.args.map(src => _srcToDstSelfIndex(indexMap, src));
+  let contextIndex = _srcToDstSelfIndex(indexMap, record.contextIndex);
+  let selfIndex = dstRecords.length + 1;
+
+  return new ProtoRecord(record.mode, record.name, record.funcOrValue, args, record.fixedArgs,
+                         contextIndex, record.directiveIndex, selfIndex, record.bindingRecord,
+                         record.lastInBinding, record.lastInDirective,
+                         record.argumentToPureFunction, record.referencedBySelf,
+                         record.propertyBindingIndex);
+}
+
+/**
+ * Returns the index in the destination array corresponding to the index in the src array.
+ * When the element is not present in the destination array, return the source index.
+ */
+function _srcToDstSelfIndex(indexMap: Map<number, number>, srcIdx: number): number {
+  var dstIdx = indexMap.get(srcIdx);
+  return isPresent(dstIdx) ? dstIdx : srcIdx;
+}
+
+function _createSelfRecord(r: ProtoRecord, contextIndex: number, selfIndex: number): ProtoRecord {
  return new ProtoRecord(RecordType.Self, "self", null, [], r.fixedArgs, contextIndex,
                         r.directiveIndex, selfIndex, r.bindingRecord, r.lastInBinding,
                         r.lastInDirective, false, false, r.propertyBindingIndex);
 }

-function _findMatching(r: ProtoRecord, rs: ProtoRecord[]) {
-  return ListWrapper.find(
-      rs, (rr) => rr.mode !== RecordType.DirectiveLifecycle && _sameDirIndex(rr, r) &&
-                  rr.mode === r.mode && looseIdentical(rr.funcOrValue, r.funcOrValue) &&
-                  rr.contextIndex === r.contextIndex && StringWrapper.equals(rr.name, r.name) &&
-                  ListWrapper.equals(rr.args, r.args));
-}
-
-function _sameDirIndex(a: ProtoRecord, b: ProtoRecord): boolean {
+function _haveSameDirIndex(a: ProtoRecord, b: ProtoRecord): boolean {
  var di1 = isBlank(a.directiveIndex) ? null : a.directiveIndex.directiveIndex;
  var ei1 = isBlank(a.directiveIndex) ? null : a.directiveIndex.elementIndex;

@ -66,17 +173,3 @@ function _sameDirIndex(a: ProtoRecord, b: ProtoRecord): boolean {

  return di1 === di2 && ei1 === ei2;
 }
-
-function _replaceIndices(r: ProtoRecord, selfIndex: number, indexMap: Map<any, any>) {
-  var args = r.args.map(a => _map(indexMap, a));
-  var contextIndex = _map(indexMap, r.contextIndex);
-  return new ProtoRecord(r.mode, r.name, r.funcOrValue, args, r.fixedArgs, contextIndex,
-                         r.directiveIndex, selfIndex, r.bindingRecord, r.lastInBinding,
-                         r.lastInDirective, r.argumentToPureFunction, r.referencedBySelf,
-                         r.propertyBindingIndex);
-}
-
-function _map(indexMap: Map<any, any>, value: number) {
-  var r = indexMap.get(value);
-  return isPresent(r) ? r : value;
-}
--- a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts
+++ b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts
@ -54,20 +54,43 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
    var values = ListWrapper.createFixedSize(eb.records.length);
    values[0] = this.values[0];

-    for (var i = 0; i < eb.records.length; ++i) {
-      var proto = eb.records[i];
-      var res = this._calculateCurrValue(proto, values, locals);
-      if (proto.lastInBinding) {
-        this._markPathAsCheckOnce(proto);
-        return res;
+    for (var protoIdx = 0; protoIdx < eb.records.length; ++protoIdx) {
+      var proto = eb.records[protoIdx];
+
+      if (proto.isSkipRecord()) {
+        protoIdx += this._computeSkipLength(protoIdx, proto, values);
      } else {
-        this._writeSelf(proto, res, values);
+        var res = this._calculateCurrValue(proto, values, locals);
+        if (proto.lastInBinding) {
+          this._markPathAsCheckOnce(proto);
+          return res;
+        } else {
+          this._writeSelf(proto, res, values);
+        }
      }
    }

    throw new BaseException("Cannot be reached");
  }

+  private _computeSkipLength(protoIndex: number, proto: ProtoRecord, values: any[]): number {
+    if (proto.mode === RecordType.SkipRecords) {
+      return proto.fixedArgs[0] - protoIndex - 1;
+    }
+
+    if (proto.mode === RecordType.SkipRecordsIf) {
+      let condition = this._readContext(proto, values);
+      return condition ? proto.fixedArgs[0] - protoIndex - 1 : 0;
+    }
+
+    if (proto.mode === RecordType.SkipRecordsIfNot) {
+      let condition = this._readContext(proto, values);
+      return condition ? 0 : proto.fixedArgs[0] - protoIndex - 1;
+    }
+
+    throw new BaseException("Cannot be reached");
+  }
+
  /** @internal */
  _markPathAsCheckOnce(proto: ProtoRecord): void {
    if (!proto.bindingRecord.isDefaultChangeDetection()) {
@ -122,8 +145,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {

    var changes = null;
    var isChanged = false;
-    for (var i = 0; i < protos.length; ++i) {
-      var proto: ProtoRecord = protos[i];
+    for (var protoIdx = 0; protoIdx < protos.length; ++protoIdx) {
+      var proto: ProtoRecord = protos[protoIdx];
      var bindingRecord = proto.bindingRecord;
      var directiveRecord = bindingRecord.directiveRecord;

@ -140,7 +163,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
        } else if (proto.name === "OnChanges" && isPresent(changes) && !throwOnChange) {
          this._getDirectiveFor(directiveRecord.directiveIndex).onChanges(changes);
        }
-
+      } else if (proto.isSkipRecord()) {
+        protoIdx += this._computeSkipLength(protoIdx, proto, this.values);
      } else {
        var change = this._check(proto, throwOnChange, this.values, this.locals);
        if (isPresent(change)) {
--- a/modules/angular2/src/core/change_detection/proto_change_detector.ts
+++ b/modules/angular2/src/core/change_detection/proto_change_detector.ts
@ -226,9 +226,28 @@ class _ConvertAstIntoProtoRecords implements AstVisitor {

  visitBinary(ast: Binary): number {
    var left = ast.left.visit(this);
-    var right = ast.right.visit(this);
-    return this._addRecord(RecordType.PrimitiveOp, _operationToPrimitiveName(ast.operation),
-                           _operationToFunction(ast.operation), [left, right], null, 0);
+    switch (ast.operation) {
+      case '&&':
+        var branchEnd = [null];
+        this._addRecord(RecordType.SkipRecordsIfNot, "SkipRecordsIfNot", null, [], branchEnd, left);
+        var right = ast.right.visit(this);
+        branchEnd[0] = right;
+        return this._addRecord(RecordType.PrimitiveOp, "cond", ChangeDetectionUtil.cond,
+                               [left, right, left], null, 0);
+
+      case '||':
+        var branchEnd = [null];
+        this._addRecord(RecordType.SkipRecordsIf, "SkipRecordsIf", null, [], branchEnd, left);
+        var right = ast.right.visit(this);
+        branchEnd[0] = right;
+        return this._addRecord(RecordType.PrimitiveOp, "cond", ChangeDetectionUtil.cond,
+                               [left, left, right], null, 0);
+
+      default:
+        var right = ast.right.visit(this);
+        return this._addRecord(RecordType.PrimitiveOp, _operationToPrimitiveName(ast.operation),
+                               _operationToFunction(ast.operation), [left, right], null, 0);
+    }
  }

  visitPrefixNot(ast: PrefixNot): number {
@ -238,11 +257,20 @@ class _ConvertAstIntoProtoRecords implements AstVisitor {
  }

  visitConditional(ast: Conditional): number {
-    var c = ast.condition.visit(this);
-    var t = ast.trueExp.visit(this);
-    var f = ast.falseExp.visit(this);
-    return this._addRecord(RecordType.PrimitiveOp, "cond", ChangeDetectionUtil.cond, [c, t, f],
-                           null, 0);
+    var condition = ast.condition.visit(this);
+    var startOfFalseBranch = [null];
+    var endOfFalseBranch = [null];
+    this._addRecord(RecordType.SkipRecordsIfNot, "SkipRecordsIfNot", null, [], startOfFalseBranch,
+                    condition);
+    var whenTrue = ast.trueExp.visit(this);
+    var skip =
+        this._addRecord(RecordType.SkipRecords, "SkipRecords", null, [], endOfFalseBranch, 0);
+    var whenFalse = ast.falseExp.visit(this);
+    startOfFalseBranch[0] = skip;
+    endOfFalseBranch[0] = whenFalse;
+
+    return this._addRecord(RecordType.PrimitiveOp, "cond", ChangeDetectionUtil.cond,
+                           [condition, whenTrue, whenFalse], null, 0);
  }

  visitPipe(ast: BindingPipe): number {
@ -350,10 +378,6 @@ function _operationToPrimitiveName(operation: string): string {
      return "operation_less_or_equals_then";
    case '>=':
      return "operation_greater_or_equals_then";
-    case '&&':
-      return "operation_logical_and";
-    case '||':
-      return "operation_logical_or";
    default:
      throw new BaseException(`Unsupported operation ${operation}`);
  }
@ -387,10 +411,6 @@ function _operationToFunction(operation: string): Function {
      return ChangeDetectionUtil.operation_less_or_equals_then;
    case '>=':
      return ChangeDetectionUtil.operation_greater_or_equals_then;
-    case '&&':
-      return ChangeDetectionUtil.operation_logical_and;
-    case '||':
-      return ChangeDetectionUtil.operation_logical_or;
    default:
      throw new BaseException(`Unsupported operation ${operation}`);
  }
--- a/modules/angular2/src/core/change_detection/proto_record.ts
+++ b/modules/angular2/src/core/change_detection/proto_record.ts
@ -18,7 +18,10 @@ export enum RecordType {
  CollectionLiteral,
  SafeMethodInvoke,
  DirectiveLifecycle,
-  Chain
+  Chain,
+  SkipRecordsIf,     // Skip records when the condition is true
+  SkipRecordsIfNot,  // Skip records when the condition is false
+  SkipRecords        // Skip records unconditionally
 }

 export class ProtoRecord {
@ -42,5 +45,15 @@ export class ProtoRecord {

  isPipeRecord(): boolean { return this.mode === RecordType.Pipe; }

+  isConditionalSkipRecord(): boolean {
+    return this.mode === RecordType.SkipRecordsIfNot || this.mode === RecordType.SkipRecordsIf;
+  }
+
+  isUnconditionalSkipRecord(): boolean { return this.mode === RecordType.SkipRecords; }
+
+  isSkipRecord(): boolean {
+    return this.isConditionalSkipRecord() || this.isUnconditionalSkipRecord();
+  }
+
  isLifeCycleRecord(): boolean { return this.mode === RecordType.DirectiveLifecycle; }
 }