feat(change_detection): modify change detectors to recompute pure functions only when their args change

Author: vsavkin

modules/change_detection/src/change_detection_jit_generator.es6

@@ -138,9 +138,10 @@ ${type}.prototype.detectChangesInRecords = function(throwOnChange) {
 }
 
 
-function bodyTemplate(localDefinitions:string, records:string):string {
+function bodyTemplate(localDefinitions:string, changeDefinitions:string, records:string):string {
  return `
 ${localDefinitions}
+${changeDefinitions}
 var ${TEMP_LOCAL};
 var ${CHANGE_LOCAL};
 var ${CHANGES_LOCAL} = [];
@@ -172,10 +173,11 @@ ${notify}
 `;
 }
 
-function referenceCheckTemplate(assignment, newValue, oldValue, addRecord, notify) {
+function referenceCheckTemplate(assignment, newValue, oldValue, change, addRecord, notify) {
  return `
 ${assignment}
 if (${newValue} !== ${oldValue} || (${newValue} !== ${newValue}) && (${oldValue} !== ${oldValue})) {
+ ${change} = true;
  ${addRecord}
  ${oldValue} = ${newValue};
 }
@@ -202,10 +204,23 @@ function localDefinitionsTemplate(names:List):string {
  return names.map((n) => `var ${n};`).join("\n");
 }
 
+function changeDefinitionsTemplate(names:List):string {
+ return names.map((n) => `var ${n} = false;`).join("\n");
+}
+
 function fieldDefinitionsTemplate(names:List):string {
  return names.map((n) => `${n} = ${UTIL}.unitialized();`).join("\n");
 }
 
+function ifChangedGuardTemplate(changeNames:List, body:string):string {
+ var cond = changeNames.join(" || ");
+ return `
+if (${cond}) {
+ ${body}
+}
+`;
+}
+
 function addSimpleChangeRecordTemplate(protoIndex:number, oldValue:string, newValue:string) {
  return `${CHANGES_LOCAL}.push(${UTIL}.simpleChangeRecord(${PROTOS_ACCESSOR}[${protoIndex}].bindingMemento, ${oldValue}, ${newValue}));`;
 }
@@ -215,13 +230,15 @@ export class ChangeDetectorJITGenerator {
  typeName:string;
  records:List<ProtoRecord>;
  localNames:List<String>;
+ changeNames:List<String>;
  fieldNames:List<String>;
 
  constructor(typeName:string, records:List<ProtoRecord>) {
  this.typeName = typeName;
  this.records = records;
 
  this.localNames = this.getLocalNames(records);
+ this.changeNames = this.getChangeNames(this.localNames);
  this.fieldNames = this.getFieldNames(this.localNames);
  }
 
@@ -234,6 +251,10 @@ export class ChangeDetectorJITGenerator {
  return ["context"].concat(names);
  }
 
+ getChangeNames(localNames:List<String>):List<String> {
+ return localNames.map((n) => `change_${n}`);
+ }
+
  getFieldNames(localNames:List<String>):List<String> {
  return localNames.map((n) => `this.${n}`);
  }
@@ -259,13 +280,17 @@ export class ChangeDetectorJITGenerator {
 
  genBody():string {
  var rec = this.records.map((r) => this.genRecord(r)).join("\n");
- return bodyTemplate(this.genLocalDefinitions(), rec);
+ return bodyTemplate(this.genLocalDefinitions(), this.genChangeDefinitions(), rec);
  }
 
  genLocalDefinitions():string {
  return localDefinitionsTemplate(this.localNames);
  }
 
+ genChangeDefinitions():string {
+ return changeDefinitionsTemplate(this.changeNames);
+ }
+
  genRecord(r:ProtoRecord):string {
  if (r.mode == RECORD_TYPE_STRUCTURAL_CHECK) {
  return this.getStructuralCheck(r);
@@ -283,10 +308,17 @@ export class ChangeDetectorJITGenerator {
  genReferenceCheck(r:ProtoRecord):string {
  var newValue = this.localNames[r.selfIndex];
  var oldValue = this.fieldNames[r.selfIndex];
+ var change = this.changeNames[r.selfIndex];
  var assignment = this.genUpdateCurrentValue(r);
  var addRecord = addSimpleChangeRecordTemplate(r.selfIndex - 1, oldValue, newValue);
  var notify = this.genNotify(r);
- return referenceCheckTemplate(assignment, newValue, oldValue, r.lastInBinding ? addRecord : '', notify);
+
+ var check = referenceCheckTemplate(assignment, newValue, oldValue, change, r.lastInBinding ? addRecord : '', notify);;
+ if (r.isPureFunction()) {
+ return this.ifChangedGuard(r, check);
+ } else {
+ return check;
+ }
  }
 
  genUpdateCurrentValue(r:ProtoRecord):string {
@@ -320,18 +352,22 @@ export class ChangeDetectorJITGenerator {
  case RECORD_TYPE_INTERPOLATE:
  return assignmentTemplate(newValue, this.genInterpolation(r));
 
+ case RECORD_TYPE_INVOKE_FORMATTER:
+ return assignmentTemplate(newValue, `${FORMATTERS_ACCESSOR}.get("${r.name}")(${args})`);
+
  case RECORD_TYPE_KEYED_ACCESS:
  var key = this.localNames[r.args[0]];
  return assignmentTemplate(newValue, `${context}[${key}]`);
 
- case RECORD_TYPE_INVOKE_FORMATTER:
- return assignmentTemplate(newValue, `${FORMATTERS_ACCESSOR}.get("${r.name}")(${args})`);
-
  default:
  throw new BaseException(`Unknown operation ${r.mode}`);
  }
  }
 
+ ifChangedGuard(r:ProtoRecord, body:string):string {
+ return ifChangedGuardTemplate(r.args.map((a) => this.changeNames[a]), body);
+ }
+
  genInterpolation(r:ProtoRecord):string{
  var res = "";
  for (var i = 0; i < r.args.length; ++i) {

modules/change_detection/src/dynamic_change_detector.js

@@ -30,14 +30,19 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
  dispatcher:any;
  formatters:Map;
  values:List;
+ changes:List;
  protos:List<ProtoRecord>;
 
  constructor(dispatcher:any, formatters:Map, protoRecords:List<ProtoRecord>) {
  super();
  this.dispatcher = dispatcher;
  this.formatters = formatters;
+
  this.values = ListWrapper.createFixedSize(protoRecords.length + 1);
  ListWrapper.fill(this.values, uninitialized);
+
+ this.changes = ListWrapper.createFixedSize(protoRecords.length + 1);
+
  this.protos = protoRecords;
  }
 
@@ -82,17 +87,25 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
  }
 
  _referenceCheck(proto:ProtoRecord) {
+ if (this._pureFuncAndArgsDidNotChange(proto)) {
+ this._setChanged(proto, false);
+ return null;
+ }
+
  var prevValue = this._readSelf(proto);
  var currValue = this._calculateCurrValue(proto);
 
  if (!isSame(prevValue, currValue)) {
  this._writeSelf(proto, currValue);
+ this._setChanged(proto, true);
+
  if (proto.lastInBinding) {
  return new SimpleChange(prevValue, currValue);
  } else {
  return null;
  }
  } else {
+ this._setChanged(proto, false);
  return null;
  }
  }
@@ -179,6 +192,24 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
  this.values[proto.selfIndex] = value;
  }
 
+ _setChanged(proto:ProtoRecord, value:boolean) {
+ this.changes[proto.selfIndex] = value;
+ }
+
+ _pureFuncAndArgsDidNotChange(proto:ProtoRecord):boolean {
+ return proto.isPureFunction() && !this._argsChanged(proto);
+ }
+
+ _argsChanged(proto:ProtoRecord):boolean {
+ var args = proto.args;
+ for(var i = 0; i < args.length; ++i) {
+ if (this.changes[args[i]]) {
+ return true;
+ }
+ }
+ return false;
+ }
+
  _readArgs(proto:ProtoRecord) {
  var res = ListWrapper.createFixedSize(proto.args.length);
  var args = proto.args;

modules/change_detection/src/proto_change_detector.js

@@ -84,6 +84,12 @@ export class ProtoRecord {
  this.lastInGroup = lastInGroup;
  this.expressionAsString = expressionAsString;
  }
+
+ isPureFunction():boolean {
+ return this.mode === RECORD_TYPE_INTERPOLATE ||
+ this.mode === RECORD_TYPE_INVOKE_FORMATTER ||
+ this.mode === RECORD_TYPE_PRIMITIVE_OP;
+ }
 }
 
 export class ProtoChangeDetector {

modules/change_detection/test/change_detection_spec.js

@@ -443,6 +443,25 @@ export function main() {
  });
  });
  });
+
+ describe("optimizations", () => {
+ it("should not rerun formatters when args did not change", () => {
+ var count = 0;
+ var formatters = MapWrapper.createFromPairs([
+ ['count', (v) => {count ++; "value"}]]);
+
+ var c = createChangeDetector('a', 'a | count', new TestData(null), formatters);
+ var cd = c["changeDetector"];
+
+ cd.detectChanges();
+
+ expect(count).toEqual(1);
+
+ cd.detectChanges();
+
+ expect(count).toEqual(1);
+ });
+ });
  });
  });
 }