Skip to content

Commit 05a1c6c

Browse files
Tim Blasimhevery
authored andcommitted
perf(compiler): Avoid unnecessary List concats
Update `BindingRecordsCreator#getBindingRecords` and `ProtoRecordBuilder#addAst` to avoid unnecessary calls to `ListWrapper.concat`. Closes angular#1905
1 parent 534cbb4 commit 05a1c6c

File tree

2 files changed

+47
-52
lines changed

2 files changed

+47
-52
lines changed

modules/angular2/src/change_detection/proto_change_detector.ts

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {isPresent, isBlank, BaseException, Type, isString} from 'angular2/src/facade/lang';
1+
import {BaseException, Type, isBlank, isPresent, isString} from 'angular2/src/facade/lang';
22
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
33

44
import {
@@ -109,37 +109,30 @@ class ProtoRecordBuilder {
109109
constructor() { this.records = []; }
110110

111111
addAst(b: BindingRecord, variableNames: List < string >= null) {
112-
var last = ListWrapper.last(this.records);
113-
if (isPresent(last) && last.bindingRecord.directiveRecord == b.directiveRecord) {
114-
last.lastInDirective = false;
112+
var oldLast = ListWrapper.last(this.records);
113+
if (isPresent(oldLast) && oldLast.bindingRecord.directiveRecord == b.directiveRecord) {
114+
oldLast.lastInDirective = false;
115115
}
116116

117-
var pr = _ConvertAstIntoProtoRecords.convert(b, this.records.length, variableNames);
118-
if (!ListWrapper.isEmpty(pr)) {
119-
var last = ListWrapper.last(pr);
120-
last.lastInBinding = true;
121-
last.lastInDirective = true;
122-
123-
this.records = ListWrapper.concat(this.records, pr);
117+
_ConvertAstIntoProtoRecords.append(this.records, b, variableNames);
118+
var newLast = ListWrapper.last(this.records);
119+
if (isPresent(newLast) && newLast !== oldLast) {
120+
newLast.lastInBinding = true;
121+
newLast.lastInDirective = true;
124122
}
125123
}
126124
}
127125

128126
class _ConvertAstIntoProtoRecords {
129-
protoRecords: List<any>;
130-
131-
constructor(private bindingRecord: BindingRecord, private contextIndex: number,
132-
private expressionAsString: string, private variableNames: List<any>) {
133-
this.protoRecords = [];
134-
}
127+
constructor(private _records: List<ProtoRecord>, private _bindingRecord: BindingRecord,
128+
private _expressionAsString: string, private _variableNames: List<any>) {}
135129

136-
static convert(b: BindingRecord, contextIndex: number, variableNames: List<any>) {
137-
var c = new _ConvertAstIntoProtoRecords(b, contextIndex, b.ast.toString(), variableNames);
130+
static append(records: List<ProtoRecord>, b: BindingRecord, variableNames: List<any>) {
131+
var c = new _ConvertAstIntoProtoRecords(records, b, b.ast.toString(), variableNames);
138132
b.ast.visit(c);
139-
return c.protoRecords;
140133
}
141134

142-
visitImplicitReceiver(ast: ImplicitReceiver) { return this.bindingRecord.implicitReceiver; }
135+
visitImplicitReceiver(ast: ImplicitReceiver) { return this._bindingRecord.implicitReceiver; }
143136

144137
visitInterpolation(ast: Interpolation) {
145138
var args = this._visitAll(ast.expressions);
@@ -153,7 +146,7 @@ class _ConvertAstIntoProtoRecords {
153146

154147
visitAccessMember(ast: AccessMember) {
155148
var receiver = ast.receiver.visit(this);
156-
if (isPresent(this.variableNames) && ListWrapper.contains(this.variableNames, ast.name) &&
149+
if (isPresent(this._variableNames) && ListWrapper.contains(this._variableNames, ast.name) &&
157150
ast.receiver instanceof
158151
ImplicitReceiver) {
159152
return this._addRecord(RECORD_TYPE_LOCAL, ast.name, ast.name, [], null, receiver);
@@ -163,10 +156,9 @@ class _ConvertAstIntoProtoRecords {
163156
}
164157

165158
visitMethodCall(ast: MethodCall) {
166-
;
167159
var receiver = ast.receiver.visit(this);
168160
var args = this._visitAll(ast.args);
169-
if (isPresent(this.variableNames) && ListWrapper.contains(this.variableNames, ast.name)) {
161+
if (isPresent(this._variableNames) && ListWrapper.contains(this._variableNames, ast.name)) {
170162
var target = this._addRecord(RECORD_TYPE_LOCAL, ast.name, ast.name, [], null, receiver);
171163
return this._addRecord(RECORD_TYPE_INVOKE_CLOSURE, "closure", null, args, null, target);
172164
} else {
@@ -236,17 +228,15 @@ class _ConvertAstIntoProtoRecords {
236228
}
237229

238230
_addRecord(type, name, funcOrValue, args, fixedArgs, context) {
239-
var selfIndex = ++this.contextIndex;
231+
var selfIndex = this._records.length + 1;
240232
if (context instanceof DirectiveIndex) {
241-
ListWrapper.push(
242-
this.protoRecords,
243-
new ProtoRecord(type, name, funcOrValue, args, fixedArgs, -1, context, selfIndex,
244-
this.bindingRecord, this.expressionAsString, false, false));
233+
ListWrapper.push(this._records, new ProtoRecord(type, name, funcOrValue, args, fixedArgs, -1,
234+
context, selfIndex, this._bindingRecord,
235+
this._expressionAsString, false, false));
245236
} else {
246-
ListWrapper.push(
247-
this.protoRecords,
248-
new ProtoRecord(type, name, funcOrValue, args, fixedArgs, context, null, selfIndex,
249-
this.bindingRecord, this.expressionAsString, false, false));
237+
ListWrapper.push(this._records, new ProtoRecord(type, name, funcOrValue, args, fixedArgs,
238+
context, null, selfIndex, this._bindingRecord,
239+
this._expressionAsString, false, false));
250240
}
251241
return selfIndex;
252242
}

modules/angular2/src/core/compiler/proto_view_factory.js

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ class BindingRecordsCreator {
2929

3030
for (var boundElementIndex = 0; boundElementIndex < elementBinders.length; boundElementIndex++) {
3131
var renderElementBinder = elementBinders[boundElementIndex];
32-
bindings = ListWrapper.concat(bindings, this._createTextNodeRecords(renderElementBinder));
33-
bindings = ListWrapper.concat(bindings, this._createElementPropertyRecords(boundElementIndex, renderElementBinder));
34-
bindings = ListWrapper.concat(bindings, this._createDirectiveRecords(boundElementIndex,
35-
renderElementBinder.directives, allDirectiveMetadatas));
32+
this._createTextNodeRecords(bindings, renderElementBinder);
33+
this._createElementPropertyRecords(bindings, boundElementIndex, renderElementBinder);
34+
this._createDirectiveRecords(bindings, boundElementIndex,
35+
renderElementBinder.directives, allDirectiveMetadatas);
3636
}
3737

3838
return bindings;
@@ -46,29 +46,35 @@ class BindingRecordsCreator {
4646
for (var elementIndex = 0; elementIndex < elementBinders.length; ++elementIndex) {
4747
var dirs = elementBinders[elementIndex].directives;
4848
for (var dirIndex = 0; dirIndex < dirs.length; ++dirIndex) {
49-
ListWrapper.push(directiveRecords, this._getDirectiveRecord(elementIndex, dirIndex, allDirectiveMetadatas[dirs[dirIndex].directiveIndex]));
49+
ListWrapper.push(directiveRecords,
50+
this._getDirectiveRecord(
51+
elementIndex, dirIndex, allDirectiveMetadatas[dirs[dirIndex].directiveIndex]));
5052
}
5153
}
5254

5355
return directiveRecords;
5456
}
5557

56-
_createTextNodeRecords(renderElementBinder:renderApi.ElementBinder) {
57-
if (isBlank(renderElementBinder.textBindings)) return [];
58-
return ListWrapper.map(renderElementBinder.textBindings, b => BindingRecord.createForTextNode(b, this._textNodeIndex++));
58+
_createTextNodeRecords(bindings: List<BindingRecord>,
59+
renderElementBinder: renderApi.ElementBinder) {
60+
if (isBlank(renderElementBinder.textBindings)) return;
61+
62+
ListWrapper.forEach(renderElementBinder.textBindings, (b) => {
63+
ListWrapper.push(bindings, BindingRecord.createForTextNode(b, this._textNodeIndex++));
64+
});
5965
}
6066

61-
_createElementPropertyRecords(boundElementIndex:number, renderElementBinder:renderApi.ElementBinder) {
62-
var res = [];
67+
_createElementPropertyRecords(bindings: List<BindingRecord>,
68+
boundElementIndex:number, renderElementBinder:renderApi.ElementBinder) {
6369
MapWrapper.forEach(renderElementBinder.propertyBindings, (astWithSource, propertyName) => {
64-
ListWrapper.push(res, BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName));
70+
ListWrapper.push(bindings,
71+
BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName));
6572
});
66-
return res;
6773
}
6874

69-
_createDirectiveRecords(boundElementIndex:number, directiveBinders:List<renderApi.DirectiveBinder>,
75+
_createDirectiveRecords(bindings: List<BindingRecord>,
76+
boundElementIndex:number, directiveBinders:List<renderApi.DirectiveBinder>,
7077
allDirectiveMetadatas:List<renderApi.DirectiveMetadata>) {
71-
var res = [];
7278
for (var i = 0; i < directiveBinders.length; i++) {
7379
var directiveBinder = directiveBinders[i];
7480
var directiveMetadata = allDirectiveMetadatas[directiveBinder.directiveIndex];
@@ -79,18 +85,17 @@ class BindingRecordsCreator {
7985
// it monomorphic!
8086
var setter = reflector.setter(propertyName);
8187
var directiveRecord = this._getDirectiveRecord(boundElementIndex, i, directiveMetadata);
82-
var b = BindingRecord.createForDirective(astWithSource, propertyName, setter, directiveRecord);
83-
ListWrapper.push(res, b);
88+
ListWrapper.push(bindings,
89+
BindingRecord.createForDirective(astWithSource, propertyName, setter, directiveRecord));
8490
});
8591

8692
// host properties
8793
MapWrapper.forEach(directiveBinder.hostPropertyBindings, (astWithSource, propertyName) => {
8894
var dirIndex = new DirectiveIndex(boundElementIndex, i);
89-
var b = BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName);
90-
ListWrapper.push(res, b);
95+
ListWrapper.push(bindings,
96+
BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName));
9197
});
9298
}
93-
return res;
9499
}
95100

96101
_getDirectiveRecord(boundElementIndex:number, directiveIndex:number, directiveMetadata:renderApi.DirectiveMetadata): DirectiveRecord {

0 commit comments

Comments
 (0)