Skip to content

Commit 71ea199

Browse files
vsavkinvsavkin
committed
perf(change_detection): removed the currentProto property
1 parent 9e7363f commit 71ea199

File tree

8 files changed

+105
-91
lines changed

8 files changed

+105
-91
lines changed

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

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
1-
import {isPresent, BaseException} from 'angular2/src/facade/lang';
1+
import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang';
22
import {List, ListWrapper} from 'angular2/src/facade/collection';
33
import {ChangeDetectionUtil} from './change_detection_util';
44
import {ChangeDetectorRef} from './change_detector_ref';
55
import {DirectiveRecord} from './directive_record';
66
import {ChangeDetector, ChangeDispatcher} from './interfaces';
7-
import {ChangeDetectionError} from './exceptions';
7+
import {
8+
ChangeDetectionError,
9+
ExpressionChangedAfterItHasBeenCheckedException,
10+
DehydratedException
11+
} from './exceptions';
812
import {ProtoRecord} from './proto_record';
13+
import {BindingRecord} from './binding_record';
914
import {Locals} from './parser/locals';
1015
import {Pipes} from './pipes/pipes';
1116
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
@@ -26,12 +31,12 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
2631
// change detection will fail.
2732
alreadyChecked: any = false;
2833
context: T;
29-
currentProto: ProtoRecord = null;
3034
directiveRecords: List<DirectiveRecord>;
3135
dispatcher: ChangeDispatcher;
3236
locals: Locals = null;
3337
mode: string = null;
3438
pipes: Pipes = null;
39+
firstProtoInCurrentBinding: number;
3540
protos: List<ProtoRecord>;
3641

3742
constructor(public id: string, dispatcher: ChangeDispatcher, protos: List<ProtoRecord>,
@@ -80,24 +85,25 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
8085
// implementation of `detectChangesInRecordsInternal` which does the work of detecting changes
8186
// and which this method will call.
8287
// This method expects that `detectChangesInRecordsInternal` will set the property
83-
// `this.currentProto` to whichever [ProtoRecord] is currently being processed. This is to
88+
// `this.firstProtoInCurrentBinding` to the selfIndex of the first proto record. This is to
8489
// facilitate error reporting.
8590
detectChangesInRecords(throwOnChange: boolean): void {
8691
if (!this.hydrated()) {
87-
ChangeDetectionUtil.throwDehydrated();
92+
this.throwDehydratedError();
8893
}
8994
try {
9095
this.detectChangesInRecordsInternal(throwOnChange);
9196
} catch (e) {
92-
this.throwError(this.currentProto, e, e.stack);
97+
this._throwError(e, e.stack);
9398
}
9499
}
95100

96101
// Subclasses should override this method to perform any work necessary to detect and report
97102
// changes. For example, changes should be reported via `ChangeDetectionUtil.addChange`, lifecycle
98103
// methods should be called, etc.
99-
// This implementation should also set `this.currentProto` to whichever [ProtoRecord] is
100-
// currently being processed to facilitate error reporting. See {@link #detectChangesInRecords}.
104+
// This implementation should also set `this.firstProtoInCurrentBinding` to the selfIndex of the
105+
// first proto record
106+
// to facilitate error reporting. See {@link #detectChangesInRecords}.
101107
detectChangesInRecordsInternal(throwOnChange: boolean): void {}
102108

103109
// This method is not intended to be overridden. Subclasses should instead provide an
@@ -155,11 +161,40 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
155161
}
156162
}
157163

158-
throwError(proto: ProtoRecord, exception: any, stack: any): void {
164+
protected notifyDispatcher(value: any): void {
165+
this.dispatcher.notifyOnBinding(this._currentBinding(), value);
166+
}
167+
168+
protected addChange(changes: StringMap<string, any>, oldValue: any,
169+
newValue: any): StringMap<string, any> {
170+
if (isBlank(changes)) {
171+
changes = {};
172+
}
173+
changes[this._currentBinding().propertyName] =
174+
ChangeDetectionUtil.simpleChange(oldValue, newValue);
175+
return changes;
176+
}
177+
178+
private _throwError(exception: any, stack: any): void {
179+
var proto = this._currentBindingProto();
159180
var c = this.dispatcher.getDebugContext(proto.bindingRecord.elementIndex, proto.directiveIndex);
160181
var context = isPresent(c) ? new _Context(c.element, c.componentElement, c.directive, c.context,
161182
c.locals, c.injector, proto.expressionAsString) :
162183
null;
163184
throw new ChangeDetectionError(proto, exception, stack, context);
164185
}
186+
187+
protected throwOnChangeError(oldValue: any, newValue: any): void {
188+
var change = ChangeDetectionUtil.simpleChange(oldValue, newValue);
189+
throw new ExpressionChangedAfterItHasBeenCheckedException(this._currentBindingProto(), change,
190+
null);
191+
}
192+
193+
protected throwDehydratedError(): void { throw new DehydratedException(); }
194+
195+
private _currentBinding(): BindingRecord { return this._currentBindingProto().bindingRecord; }
196+
197+
private _currentBindingProto(): ProtoRecord {
198+
return ChangeDetectionUtil.protoByIndex(this.protos, this.firstProtoInCurrentBinding);
199+
}
165200
}

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

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ export class ChangeDetectorJITGenerator {
4646
${this._typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype);
4747
4848
${this._typeName}.prototype.detectChangesInRecordsInternal = function(throwOnChange) {
49-
${this._names.getCurrentProtoName()} = null;
50-
5149
${this._names.genInitLocals()}
5250
var ${IS_CHANGED_LOCAL} = false;
5351
var ${CHANGES_LOCAL} = null;
@@ -149,7 +147,11 @@ export class ChangeDetectorJITGenerator {
149147
} else {
150148
rec = this._genReferenceCheck(r);
151149
}
152-
return `${rec}${this._maybeGenLastInDirective(r)}`;
150+
return `
151+
${this._maybeFirstInBinding(r)}
152+
${rec}
153+
${this._maybeGenLastInDirective(r)}
154+
`;
153155
}
154156

155157
_genDirectiveLifecycle(r: ProtoRecord): string {
@@ -173,12 +175,9 @@ export class ChangeDetectorJITGenerator {
173175

174176
var pipe = this._names.getPipeName(r.selfIndex);
175177
var cdRef = "this.ref";
176-
177-
var protoIndex = r.selfIndex - 1;
178178
var pipeType = r.name;
179179

180180
var read = `
181-
${this._names.getCurrentProtoName()} = ${this._names.getProtosName()}[${protoIndex}];
182181
if (${pipe} === ${UTIL}.uninitialized) {
183182
${pipe} = ${this._names.getPipesAccessorName()}.get('${pipeType}', ${context}, ${cdRef});
184183
} else if (!${pipe}.supports(${context})) {
@@ -204,11 +203,7 @@ export class ChangeDetectorJITGenerator {
204203
_genReferenceCheck(r: ProtoRecord): string {
205204
var oldValue = this._names.getFieldName(r.selfIndex);
206205
var newValue = this._names.getLocalName(r.selfIndex);
207-
208-
var protoIndex = r.selfIndex - 1;
209-
210206
var read = `
211-
${this._names.getCurrentProtoName()} = ${this._names.getProtosName()}[${protoIndex}];
212207
${this._genUpdateCurrentValue(r)}
213208
`;
214209

@@ -331,8 +326,7 @@ export class ChangeDetectorJITGenerator {
331326
} else {
332327
return `
333328
${this._genThrowOnChangeCheck(oldValue, newValue)}
334-
${this._names.getDispatcherName()}.notifyOnBinding(
335-
${this._names.getCurrentProtoName()}.bindingRecord, ${newValue});
329+
this.notifyDispatcher(${newValue});
336330
`;
337331
}
338332
}
@@ -341,7 +335,7 @@ export class ChangeDetectorJITGenerator {
341335
if (this.generateCheckNoChanges) {
342336
return `
343337
if(throwOnChange) {
344-
${UTIL}.throwOnChange(${this._names.getCurrentProtoName()}, ${UTIL}.simpleChange(${oldValue}, ${newValue}));
338+
this.throwOnChangeError(${oldValue}, ${newValue});
345339
}
346340
`;
347341
} else {
@@ -361,11 +355,13 @@ export class ChangeDetectorJITGenerator {
361355
var newValue = this._names.getLocalName(r.selfIndex);
362356
var oldValue = this._names.getFieldName(r.selfIndex);
363357
if (!r.bindingRecord.callOnChange()) return "";
364-
return `
365-
${CHANGES_LOCAL} = ${UTIL}.addChange(
366-
${CHANGES_LOCAL}, ${this._names.getCurrentProtoName()}.bindingRecord.propertyName,
367-
${UTIL}.simpleChange(${oldValue}, ${newValue}));
368-
`;
358+
return `${CHANGES_LOCAL} = this.addChange(${CHANGES_LOCAL}, ${oldValue}, ${newValue});`;
359+
}
360+
361+
_maybeFirstInBinding(r: ProtoRecord): string {
362+
var prev = ChangeDetectionUtil.protoByIndex(this.records, r.selfIndex - 1);
363+
var firstInBindng = isBlank(prev) || prev.bindingRecord !== r.bindingRecord;
364+
return firstInBindng ? `${this._names.getFirstProtoInCurrentBinding()} = ${r.selfIndex};` : '';
369365
}
370366

371367
_maybeGenLastInDirective(r: ProtoRecord): string {

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

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {CONST_EXPR, isPresent, isBlank, BaseException, Type} from 'angular2/src/facade/lang';
22
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
33
import {ProtoRecord} from './proto_record';
4-
import {DehydratedException, ExpressionChangedAfterItHasBeenCheckedException} from './exceptions';
54
import {WrappedValue} from './pipes/pipe';
65
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
76

@@ -126,12 +125,6 @@ export class ChangeDetectionUtil {
126125
}
127126
}
128127

129-
static throwOnChange(proto: ProtoRecord, change) {
130-
throw new ExpressionChangedAfterItHasBeenCheckedException(proto, change, null);
131-
}
132-
133-
static throwDehydrated() { throw new DehydratedException(); }
134-
135128
static changeDetectionMode(strategy: string): string {
136129
return strategy == ON_PUSH ? CHECK_ONCE : CHECK_ALWAYS;
137130
}
@@ -140,15 +133,13 @@ export class ChangeDetectionUtil {
140133
return _simpleChange(previousValue, currentValue);
141134
}
142135

143-
static addChange(changes, propertyName: string, change): Map<any, any> {
144-
if (isBlank(changes)) {
145-
changes = {};
146-
}
147-
changes[propertyName] = change;
148-
return changes;
149-
}
150-
151136
static isValueBlank(value: any): boolean { return isBlank(value); }
152137

153138
static s(value: any): string { return isPresent(value) ? `${value}` : ''; }
139+
140+
static protoByIndex(protos: ProtoRecord[], selfIndex: number): ProtoRecord {
141+
return selfIndex < 1 ?
142+
null :
143+
protos[selfIndex - 1]; // self index is shifted by one because of context
144+
}
154145
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {ProtoRecord} from './proto_record';
99
// detection will fail.
1010
const _ALREADY_CHECKED_ACCESSOR = "alreadyChecked";
1111
const _CONTEXT_ACCESSOR = "context";
12-
const _CURRENT_PROTO = "currentProto";
12+
const _FIRST_PROTO_IN_CURRENT_BINDING = "firstProtoInCurrentBinding";
1313
const _DIRECTIVES_ACCESSOR = "directiveRecords";
1414
const _DISPATCHER_ACCESSOR = "dispatcher";
1515
const _LOCALS_ACCESSOR = "locals";
@@ -67,7 +67,9 @@ export class CodegenNameUtil {
6767

6868
getModeName(): string { return this._addFieldPrefix(_MODE_ACCESSOR); }
6969

70-
getCurrentProtoName(): string { return this._addFieldPrefix(_CURRENT_PROTO); }
70+
getFirstProtoInCurrentBinding(): string {
71+
return this._addFieldPrefix(_FIRST_PROTO_IN_CURRENT_BINDING);
72+
}
7173

7274
getLocalName(idx: int): string { return `l_${this._sanitizedNames[idx]}`; }
7375

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
5555

5656
checkNoChanges(): void { this.runDetectChanges(true); }
5757

58-
// TODO(vsavkin): #3366. Update to work with [AbstractChangeDetector#detectChangesInRecords]
59-
detectChangesInRecords(throwOnChange: boolean) {
60-
if (!this.hydrated()) {
61-
ChangeDetectionUtil.throwDehydrated();
62-
}
63-
var protos: List<ProtoRecord> = this.protos;
58+
detectChangesInRecordsInternal(throwOnChange: boolean) {
59+
var protos = this.protos;
6460

6561
var changes = null;
6662
var isChanged = false;
@@ -69,6 +65,10 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
6965
var bindingRecord = proto.bindingRecord;
7066
var directiveRecord = bindingRecord.directiveRecord;
7167

68+
if (this._firstInBinding(proto)) {
69+
this.firstProtoInCurrentBinding = proto.selfIndex;
70+
}
71+
7272
if (proto.isLifeCycleRecord()) {
7373
if (proto.name === "onCheck" && !throwOnChange) {
7474
this._getDirectiveFor(directiveRecord.directiveIndex).onCheck();
@@ -100,6 +100,11 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
100100
this.alreadyChecked = true;
101101
}
102102

103+
_firstInBinding(r: ProtoRecord): boolean {
104+
var prev = ChangeDetectionUtil.protoByIndex(this.protos, r.selfIndex - 1);
105+
return isBlank(prev) || prev.bindingRecord !== r.bindingRecord;
106+
}
107+
103108
callOnAllChangesDone() {
104109
this.dispatcher.notifyOnAllChangesDone();
105110
var dirs = this.directiveRecords;
@@ -122,7 +127,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
122127

123128
_addChange(bindingRecord: BindingRecord, change, changes) {
124129
if (bindingRecord.callOnChange()) {
125-
return ChangeDetectionUtil.addChange(changes, bindingRecord.propertyName, change);
130+
return super.addChange(changes, change.previousValue, change.currentValue);
126131
} else {
127132
return changes;
128133
}
@@ -133,14 +138,10 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
133138
_getDetectorFor(directiveIndex) { return this.directives.getDetectorFor(directiveIndex); }
134139

135140
_check(proto: ProtoRecord, throwOnChange: boolean): SimpleChange {
136-
try {
137-
if (proto.isPipeRecord()) {
138-
return this._pipeCheck(proto, throwOnChange);
139-
} else {
140-
return this._referenceCheck(proto, throwOnChange);
141-
}
142-
} catch (e) {
143-
this.throwError(proto, e, e.stack);
141+
if (proto.isPipeRecord()) {
142+
return this._pipeCheck(proto, throwOnChange);
143+
} else {
144+
return this._referenceCheck(proto, throwOnChange);
144145
}
145146
}
146147

@@ -156,7 +157,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
156157
if (!isSame(prevValue, currValue)) {
157158
if (proto.lastInBinding) {
158159
var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
159-
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
160+
if (throwOnChange) this.throwOnChangeError(prevValue, currValue);
160161

161162
this._writeSelf(proto, currValue);
162163
this._setChanged(proto, true);
@@ -241,7 +242,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
241242

242243
if (proto.lastInBinding) {
243244
var change = ChangeDetectionUtil.simpleChange(prevValue, currValue);
244-
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
245+
if (throwOnChange) this.throwOnChangeError(prevValue, currValue);
245246

246247
this._writeSelf(proto, currValue);
247248
this._setChanged(proto, true);

0 commit comments

Comments
 (0)