Skip to content

Commit c1ee943

Browse files
vsavkinvsavkin
committed
perf(change_detection): do not check intermediate results
1 parent f7d7789 commit c1ee943

13 files changed

Lines changed: 300 additions & 71 deletions

File tree

‎modules/angular2/src/change_detection/change_detection_jit_generator.ts‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ export class ChangeDetectorJITGenerator {
7171
return new ${this._typeName}(dispatcher, protos, directiveRecords);
7272
}
7373
`;
74-
7574
return new Function('AbstractChangeDetector', 'ChangeDetectionUtil', 'protos',
7675
'directiveRecords', classDefinition)(
7776
AbstractChangeDetector, ChangeDetectionUtil, this.records, this.directiveRecords);
@@ -171,55 +170,68 @@ export class ChangeDetectorJITGenerator {
171170

172171
var oldValue = this._names.getFieldName(r.selfIndex);
173172
var newValue = this._names.getLocalName(r.selfIndex);
174-
var change = this._names.getChangeName(r.selfIndex);
175173

176174
var pipe = this._names.getPipeName(r.selfIndex);
177175
var cdRef = "this.ref";
178176

179177
var protoIndex = r.selfIndex - 1;
180178
var pipeType = r.name;
181179

182-
return `
180+
var read = `
183181
${this._names.getCurrentProtoName()} = ${this._names.getProtosName()}[${protoIndex}];
184182
if (${pipe} === ${UTIL}.uninitialized) {
185183
${pipe} = ${this._names.getPipesAccessorName()}.get('${pipeType}', ${context}, ${cdRef});
186184
} else if (!${pipe}.supports(${context})) {
187185
${pipe}.onDestroy();
188186
${pipe} = ${this._names.getPipesAccessorName()}.get('${pipeType}', ${context}, ${cdRef});
189187
}
190-
191188
${newValue} = ${pipe}.transform(${context}, [${argString}]);
189+
`;
190+
191+
var check = `
192192
if (${oldValue} !== ${newValue}) {
193-
${newValue} = ${UTIL}.unwrapValue(${newValue});
194-
${change} = true;
193+
${newValue} = ${UTIL}.unwrapValue(${newValue})
194+
${this._genChangeMarker(r)}
195195
${this._genUpdateDirectiveOrElement(r)}
196196
${this._genAddToChanges(r)}
197197
${oldValue} = ${newValue};
198198
}
199199
`;
200+
201+
return r.shouldBeChecked() ? `${read}${check}` : read;
200202
}
201203

202204
_genReferenceCheck(r: ProtoRecord): string {
203205
var oldValue = this._names.getFieldName(r.selfIndex);
204206
var newValue = this._names.getLocalName(r.selfIndex);
205207

206208
var protoIndex = r.selfIndex - 1;
207-
var check = `
209+
210+
var read = `
208211
${this._names.getCurrentProtoName()} = ${this._names.getProtosName()}[${protoIndex}];
209212
${this._genUpdateCurrentValue(r)}
213+
`;
214+
215+
var check = `
210216
if (${newValue} !== ${oldValue}) {
211-
${this._names.getChangeName(r.selfIndex)} = true;
217+
${this._genChangeMarker(r)}
212218
${this._genUpdateDirectiveOrElement(r)}
213219
${this._genAddToChanges(r)}
214220
${oldValue} = ${newValue};
215221
}
216222
`;
217223

224+
var genCode = r.shouldBeChecked() ? `${read}${check}` : read;
225+
218226
if (r.isPureFunction()) {
219227
var condition = r.args.map((a) => this._names.getChangeName(a)).join(" || ");
220-
return `if (${condition}) { ${check} } else { ${newValue} = ${oldValue}; }`;
228+
if (r.isUsedByOtherRecord()) {
229+
return `if (${condition}) { ${genCode} } else { ${newValue} = ${oldValue}; }`;
230+
} else {
231+
return `if (${condition}) { ${genCode} }`;
232+
}
221233
} else {
222-
return check;
234+
return genCode;
223235
}
224236
}
225237

@@ -267,6 +279,10 @@ export class ChangeDetectorJITGenerator {
267279
rhs = `${UTIL}.${r.name}(${argString})`;
268280
break;
269281

282+
case RecordType.COLLECTION_LITERAL:
283+
rhs = `${UTIL}.${r.name}(${argString})`;
284+
break;
285+
270286
case RecordType.INTERPOLATE:
271287
rhs = this._genInterpolation(r);
272288
break;
@@ -293,6 +309,10 @@ export class ChangeDetectorJITGenerator {
293309
return res;
294310
}
295311

312+
_genChangeMarker(r: ProtoRecord): string {
313+
return r.argumentToPureFunction ? `${this._names.getChangeName(r.selfIndex)} = true` : ``;
314+
}
315+
296316
_genUpdateDirectiveOrElement(r: ProtoRecord): string {
297317
if (!r.lastInBinding) return "";
298318

‎modules/angular2/src/change_detection/coalesce.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {RecordType, ProtoRecord} from './proto_record';
1212
* Records that are last in bindings CANNOT be removed, and instead are
1313
* replaced with very cheap SELF records.
1414
*/
15-
export function coalesce(records: List<ProtoRecord>): List<ProtoRecord> {
15+
export function coalesce(records: ProtoRecord[]): ProtoRecord[] {
1616
var res: List<ProtoRecord> = [];
1717
var indexMap: Map<number, number> = new Map<number, number>();
1818

@@ -24,8 +24,13 @@ export function coalesce(records: List<ProtoRecord>): List<ProtoRecord> {
2424
if (isPresent(matchingRecord) && record.lastInBinding) {
2525
res.push(_selfRecord(record, matchingRecord.selfIndex, res.length + 1));
2626
indexMap.set(r.selfIndex, matchingRecord.selfIndex);
27+
matchingRecord.referencedBySelf = true;
2728

2829
} else if (isPresent(matchingRecord) && !record.lastInBinding) {
30+
if (record.argumentToPureFunction) {
31+
matchingRecord.argumentToPureFunction = true;
32+
}
33+
2934
indexMap.set(r.selfIndex, matchingRecord.selfIndex);
3035

3136
} else {
@@ -40,7 +45,7 @@ export function coalesce(records: List<ProtoRecord>): List<ProtoRecord> {
4045
function _selfRecord(r: ProtoRecord, contextIndex: number, selfIndex: number): ProtoRecord {
4146
return new ProtoRecord(RecordType.SELF, "self", null, [], r.fixedArgs, contextIndex,
4247
r.directiveIndex, selfIndex, r.bindingRecord, r.expressionAsString,
43-
r.lastInBinding, r.lastInDirective);
48+
r.lastInBinding, r.lastInDirective, false, false);
4449
}
4550

4651
function _findMatching(r: ProtoRecord, rs: List<ProtoRecord>) {
@@ -66,7 +71,8 @@ function _replaceIndices(r: ProtoRecord, selfIndex: number, indexMap: Map<any, a
6671
var contextIndex = _map(indexMap, r.contextIndex);
6772
return new ProtoRecord(r.mode, r.name, r.funcOrValue, args, r.fixedArgs, contextIndex,
6873
r.directiveIndex, selfIndex, r.bindingRecord, r.expressionAsString,
69-
r.lastInBinding, r.lastInDirective);
74+
r.lastInBinding, r.lastInDirective, r.argumentToPureFunction,
75+
r.referencedBySelf);
7076
}
7177

7278
function _map(indexMap: Map<any, any>, value: number) {

‎modules/angular2/src/change_detection/codegen_name_util.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,14 @@ export class CodegenNameUtil {
8383
if (i == CONTEXT_INDEX) {
8484
declarations.push(`${this.getLocalName(i)} = ${this.getFieldName(i)}`);
8585
} else {
86-
var changeName = this.getChangeName(i);
87-
declarations.push(`${this.getLocalName(i)},${changeName}`);
88-
assignments.push(changeName);
86+
var rec = this.records[i - 1];
87+
if (rec.argumentToPureFunction) {
88+
var changeName = this.getChangeName(i);
89+
declarations.push(`${this.getLocalName(i)},${changeName}`);
90+
assignments.push(changeName);
91+
} else {
92+
declarations.push(`${this.getLocalName(i)}`);
93+
}
8994
}
9095
}
9196
var assignmentsCode =
@@ -100,14 +105,18 @@ export class CodegenNameUtil {
100105
getAllFieldNames(): List<string> {
101106
var fieldList = [];
102107
for (var k = 0, kLen = this.getFieldCount(); k < kLen; ++k) {
103-
fieldList.push(this.getFieldName(k));
108+
if (k === 0 || this.records[k - 1].shouldBeChecked()) {
109+
fieldList.push(this.getFieldName(k));
110+
}
104111
}
112+
105113
for (var i = 0, iLen = this.records.length; i < iLen; ++i) {
106114
var rec = this.records[i];
107115
if (rec.isPipeRecord()) {
108116
fieldList.push(this.getPipeName(rec.selfIndex));
109117
}
110118
}
119+
111120
for (var j = 0, jLen = this.directiveRecords.length; j < jLen; ++j) {
112121
var dRec = this.directiveRecords[j];
113122
fieldList.push(this.getDirectiveName(dRec.directiveIndex));

‎modules/angular2/src/change_detection/dynamic_change_detector.ts‎

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -150,26 +150,30 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
150150
return null;
151151
}
152152

153-
var prevValue = this._readSelf(proto);
154153
var currValue = this._calculateCurrValue(proto);
155-
156-
if (!isSame(prevValue, currValue)) {
157-
if (proto.lastInBinding) {
158-
var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
159-
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
160-
161-
this._writeSelf(proto, currValue);
162-
this._setChanged(proto, true);
163-
164-
return change;
165-
154+
if (proto.shouldBeChecked()) {
155+
var prevValue = this._readSelf(proto);
156+
if (!isSame(prevValue, currValue)) {
157+
if (proto.lastInBinding) {
158+
var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
159+
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
160+
161+
this._writeSelf(proto, currValue);
162+
this._setChanged(proto, true);
163+
return change;
164+
} else {
165+
this._writeSelf(proto, currValue);
166+
this._setChanged(proto, true);
167+
return null;
168+
}
166169
} else {
167-
this._writeSelf(proto, currValue);
168-
this._setChanged(proto, true);
170+
this._setChanged(proto, false);
169171
return null;
170172
}
173+
171174
} else {
172-
this._setChanged(proto, false);
175+
this._writeSelf(proto, currValue);
176+
this._setChanged(proto, true);
173177
return null;
174178
}
175179
}
@@ -215,6 +219,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
215219

216220
case RecordType.INTERPOLATE:
217221
case RecordType.PRIMITIVE_OP:
222+
case RecordType.COLLECTION_LITERAL:
218223
return FunctionWrapper.apply(proto.funcOrValue, this._readArgs(proto));
219224

220225
default:
@@ -227,28 +232,34 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
227232
var args = this._readArgs(proto);
228233

229234
var pipe = this._pipeFor(proto, context);
230-
var prevValue = this._readSelf(proto);
231235
var currValue = pipe.transform(context, args);
232236

233-
if (!isSame(prevValue, currValue)) {
234-
currValue = ChangeDetectionUtil.unwrapValue(currValue);
237+
if (proto.shouldBeChecked()) {
238+
var prevValue = this._readSelf(proto);
239+
if (!isSame(prevValue, currValue)) {
240+
currValue = ChangeDetectionUtil.unwrapValue(currValue);
235241

236-
if (proto.lastInBinding) {
237-
var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
238-
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
242+
if (proto.lastInBinding) {
243+
var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
244+
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
239245

240-
this._writeSelf(proto, currValue);
241-
this._setChanged(proto, true);
246+
this._writeSelf(proto, currValue);
247+
this._setChanged(proto, true);
242248

243-
return change;
249+
return change;
244250

251+
} else {
252+
this._writeSelf(proto, currValue);
253+
this._setChanged(proto, true);
254+
return null;
255+
}
245256
} else {
246-
this._writeSelf(proto, currValue);
247-
this._setChanged(proto, true);
257+
this._setChanged(proto, false);
248258
return null;
249259
}
250260
} else {
251-
this._setChanged(proto, false);
261+
this._writeSelf(proto, currValue);
262+
this._setChanged(proto, true);
252263
return null;
253264
}
254265
}
@@ -284,7 +295,9 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
284295

285296
_writePipe(proto: ProtoRecord, value) { this.localPipes[proto.selfIndex] = value; }
286297

287-
_setChanged(proto: ProtoRecord, value: boolean) { this.changes[proto.selfIndex] = value; }
298+
_setChanged(proto: ProtoRecord, value: boolean) {
299+
if (proto.argumentToPureFunction) this.changes[proto.selfIndex] = value;
300+
}
288301

289302
_pureFuncAndArgsDidNotChange(proto: ProtoRecord): boolean {
290303
return proto.isPureFunction() && !this._argsChanged(proto);

‎modules/angular2/src/change_detection/proto_change_detector.ts‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,31 @@ export class ProtoRecordBuilder {
6565
if (isPresent(oldLast) && oldLast.bindingRecord.directiveRecord == b.directiveRecord) {
6666
oldLast.lastInDirective = false;
6767
}
68+
var numberOfRecordsBefore = this.records.length;
6869
this._appendRecords(b, variableNames);
6970
var newLast = ListWrapper.last(this.records);
7071
if (isPresent(newLast) && newLast !== oldLast) {
7172
newLast.lastInBinding = true;
7273
newLast.lastInDirective = true;
74+
this._setArgumentToPureFunction(numberOfRecordsBefore);
75+
}
76+
}
77+
78+
_setArgumentToPureFunction(startIndex: number): void {
79+
for (var i = startIndex; i < this.records.length; ++i) {
80+
var rec = this.records[i];
81+
if (rec.isPureFunction()) {
82+
rec.args.forEach(recordIndex => this.records[recordIndex - 1].argumentToPureFunction =
83+
true);
84+
}
7385
}
7486
}
7587

7688
_appendRecords(b: BindingRecord, variableNames: List<string>) {
7789
if (b.isDirectiveLifecycle()) {
7890
this.records.push(new ProtoRecord(RecordType.DIRECTIVE_LIFECYCLE, b.lifecycleEvent, null, [],
7991
[], -1, null, this.records.length + 1, b, null, false,
80-
false));
92+
false, false, false));
8193
} else {
8294
_ConvertAstIntoProtoRecords.append(this.records, b, variableNames);
8395
}
@@ -145,12 +157,13 @@ class _ConvertAstIntoProtoRecords implements AstVisitor {
145157

146158
visitLiteralArray(ast: LiteralArray): number {
147159
var primitiveName = `arrayFn${ast.expressions.length}`;
148-
return this._addRecord(RecordType.PRIMITIVE_OP, primitiveName, _arrayFn(ast.expressions.length),
149-
this._visitAll(ast.expressions), null, 0);
160+
return this._addRecord(RecordType.COLLECTION_LITERAL, primitiveName,
161+
_arrayFn(ast.expressions.length), this._visitAll(ast.expressions), null,
162+
0);
150163
}
151164

152165
visitLiteralMap(ast: LiteralMap): number {
153-
return this._addRecord(RecordType.PRIMITIVE_OP, _mapPrimitiveName(ast.keys),
166+
return this._addRecord(RecordType.COLLECTION_LITERAL, _mapPrimitiveName(ast.keys),
154167
ChangeDetectionUtil.mapFn(ast.keys), this._visitAll(ast.values), null,
155168
0);
156169
}
@@ -208,11 +221,11 @@ class _ConvertAstIntoProtoRecords implements AstVisitor {
208221
if (context instanceof DirectiveIndex) {
209222
this._records.push(new ProtoRecord(type, name, funcOrValue, args, fixedArgs, -1, context,
210223
selfIndex, this._bindingRecord, this._expressionAsString,
211-
false, false));
224+
false, false, false, false));
212225
} else {
213226
this._records.push(new ProtoRecord(type, name, funcOrValue, args, fixedArgs, context, null,
214227
selfIndex, this._bindingRecord, this._expressionAsString,
215-
false, false));
228+
false, false, false, false));
216229
}
217230
return selfIndex;
218231
}

‎modules/angular2/src/change_detection/proto_record.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export enum RecordType {
1414
PIPE,
1515
INTERPOLATE,
1616
SAFE_PROPERTY,
17+
COLLECTION_LITERAL,
1718
SAFE_INVOKE_METHOD,
1819
DIRECTIVE_LIFECYCLE
1920
}
@@ -23,10 +24,17 @@ export class ProtoRecord {
2324
public args: List<any>, public fixedArgs: List<any>, public contextIndex: number,
2425
public directiveIndex: DirectiveIndex, public selfIndex: number,
2526
public bindingRecord: BindingRecord, public expressionAsString: string,
26-
public lastInBinding: boolean, public lastInDirective: boolean) {}
27+
public lastInBinding: boolean, public lastInDirective: boolean,
28+
public argumentToPureFunction: boolean, public referencedBySelf: boolean) {}
2729

2830
isPureFunction(): boolean {
29-
return this.mode === RecordType.INTERPOLATE || this.mode === RecordType.PRIMITIVE_OP;
31+
return this.mode === RecordType.INTERPOLATE || this.mode === RecordType.COLLECTION_LITERAL;
32+
}
33+
34+
isUsedByOtherRecord(): boolean { return !this.lastInBinding || this.referencedBySelf; }
35+
36+
shouldBeChecked(): boolean {
37+
return this.argumentToPureFunction || this.lastInBinding || this.isPureFunction();
3038
}
3139

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

0 commit comments

Comments
 (0)