From 05a1c6c183780264d9d2aff0380d7f526ad56603 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Thu, 14 May 2015 10:26:19 -0700 Subject: [PATCH] perf(compiler): Avoid unnecessary List concats Update `BindingRecordsCreator#getBindingRecords` and `ProtoRecordBuilder#addAst` to avoid unnecessary calls to `ListWrapper.concat`. Closes #1905 --- .../change_detection/proto_change_detector.ts | 56 ++++++++----------- .../src/core/compiler/proto_view_factory.js | 47 +++++++++------- 2 files changed, 49 insertions(+), 54 deletions(-) diff --git a/modules/angular2/src/change_detection/proto_change_detector.ts b/modules/angular2/src/change_detection/proto_change_detector.ts index 5cc602ed22..ff2451ddf2 100644 --- a/modules/angular2/src/change_detection/proto_change_detector.ts +++ b/modules/angular2/src/change_detection/proto_change_detector.ts @@ -1,4 +1,4 @@ -import {isPresent, isBlank, BaseException, Type, isString} from 'angular2/src/facade/lang'; +import {BaseException, Type, isBlank, isPresent, isString} from 'angular2/src/facade/lang'; import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import { @@ -109,37 +109,30 @@ class ProtoRecordBuilder { constructor() { this.records = []; } addAst(b: BindingRecord, variableNames: List < string >= null) { - var last = ListWrapper.last(this.records); - if (isPresent(last) && last.bindingRecord.directiveRecord == b.directiveRecord) { - last.lastInDirective = false; + var oldLast = ListWrapper.last(this.records); + if (isPresent(oldLast) && oldLast.bindingRecord.directiveRecord == b.directiveRecord) { + oldLast.lastInDirective = false; } - var pr = _ConvertAstIntoProtoRecords.convert(b, this.records.length, variableNames); - if (!ListWrapper.isEmpty(pr)) { - var last = ListWrapper.last(pr); - last.lastInBinding = true; - last.lastInDirective = true; - - this.records = ListWrapper.concat(this.records, pr); + _ConvertAstIntoProtoRecords.append(this.records, b, variableNames); + var newLast = ListWrapper.last(this.records); + if (isPresent(newLast) && newLast !== oldLast) { + newLast.lastInBinding = true; + newLast.lastInDirective = true; } } } class _ConvertAstIntoProtoRecords { - protoRecords: List; + constructor(private _records: List, private _bindingRecord: BindingRecord, + private _expressionAsString: string, private _variableNames: List) {} - constructor(private bindingRecord: BindingRecord, private contextIndex: number, - private expressionAsString: string, private variableNames: List) { - this.protoRecords = []; - } - - static convert(b: BindingRecord, contextIndex: number, variableNames: List) { - var c = new _ConvertAstIntoProtoRecords(b, contextIndex, b.ast.toString(), variableNames); + static append(records: List, b: BindingRecord, variableNames: List) { + var c = new _ConvertAstIntoProtoRecords(records, b, b.ast.toString(), variableNames); b.ast.visit(c); - return c.protoRecords; } - visitImplicitReceiver(ast: ImplicitReceiver) { return this.bindingRecord.implicitReceiver; } + visitImplicitReceiver(ast: ImplicitReceiver) { return this._bindingRecord.implicitReceiver; } visitInterpolation(ast: Interpolation) { var args = this._visitAll(ast.expressions); @@ -153,7 +146,7 @@ class _ConvertAstIntoProtoRecords { visitAccessMember(ast: AccessMember) { var receiver = ast.receiver.visit(this); - if (isPresent(this.variableNames) && ListWrapper.contains(this.variableNames, ast.name) && + if (isPresent(this._variableNames) && ListWrapper.contains(this._variableNames, ast.name) && ast.receiver instanceof ImplicitReceiver) { return this._addRecord(RECORD_TYPE_LOCAL, ast.name, ast.name, [], null, receiver); @@ -163,10 +156,9 @@ class _ConvertAstIntoProtoRecords { } visitMethodCall(ast: MethodCall) { - ; var receiver = ast.receiver.visit(this); var args = this._visitAll(ast.args); - if (isPresent(this.variableNames) && ListWrapper.contains(this.variableNames, ast.name)) { + if (isPresent(this._variableNames) && ListWrapper.contains(this._variableNames, ast.name)) { var target = this._addRecord(RECORD_TYPE_LOCAL, ast.name, ast.name, [], null, receiver); return this._addRecord(RECORD_TYPE_INVOKE_CLOSURE, "closure", null, args, null, target); } else { @@ -236,17 +228,15 @@ class _ConvertAstIntoProtoRecords { } _addRecord(type, name, funcOrValue, args, fixedArgs, context) { - var selfIndex = ++this.contextIndex; + var selfIndex = this._records.length + 1; if (context instanceof DirectiveIndex) { - ListWrapper.push( - this.protoRecords, - new ProtoRecord(type, name, funcOrValue, args, fixedArgs, -1, context, selfIndex, - this.bindingRecord, this.expressionAsString, false, false)); + ListWrapper.push(this._records, new ProtoRecord(type, name, funcOrValue, args, fixedArgs, -1, + context, selfIndex, this._bindingRecord, + this._expressionAsString, false, false)); } else { - ListWrapper.push( - this.protoRecords, - new ProtoRecord(type, name, funcOrValue, args, fixedArgs, context, null, selfIndex, - this.bindingRecord, this.expressionAsString, false, false)); + ListWrapper.push(this._records, new ProtoRecord(type, name, funcOrValue, args, fixedArgs, + context, null, selfIndex, this._bindingRecord, + this._expressionAsString, false, false)); } return selfIndex; } diff --git a/modules/angular2/src/core/compiler/proto_view_factory.js b/modules/angular2/src/core/compiler/proto_view_factory.js index e2ef2679cf..f22d252202 100644 --- a/modules/angular2/src/core/compiler/proto_view_factory.js +++ b/modules/angular2/src/core/compiler/proto_view_factory.js @@ -29,10 +29,10 @@ class BindingRecordsCreator { for (var boundElementIndex = 0; boundElementIndex < elementBinders.length; boundElementIndex++) { var renderElementBinder = elementBinders[boundElementIndex]; - bindings = ListWrapper.concat(bindings, this._createTextNodeRecords(renderElementBinder)); - bindings = ListWrapper.concat(bindings, this._createElementPropertyRecords(boundElementIndex, renderElementBinder)); - bindings = ListWrapper.concat(bindings, this._createDirectiveRecords(boundElementIndex, - renderElementBinder.directives, allDirectiveMetadatas)); + this._createTextNodeRecords(bindings, renderElementBinder); + this._createElementPropertyRecords(bindings, boundElementIndex, renderElementBinder); + this._createDirectiveRecords(bindings, boundElementIndex, + renderElementBinder.directives, allDirectiveMetadatas); } return bindings; @@ -46,29 +46,35 @@ class BindingRecordsCreator { for (var elementIndex = 0; elementIndex < elementBinders.length; ++elementIndex) { var dirs = elementBinders[elementIndex].directives; for (var dirIndex = 0; dirIndex < dirs.length; ++dirIndex) { - ListWrapper.push(directiveRecords, this._getDirectiveRecord(elementIndex, dirIndex, allDirectiveMetadatas[dirs[dirIndex].directiveIndex])); + ListWrapper.push(directiveRecords, + this._getDirectiveRecord( + elementIndex, dirIndex, allDirectiveMetadatas[dirs[dirIndex].directiveIndex])); } } return directiveRecords; } - _createTextNodeRecords(renderElementBinder:renderApi.ElementBinder) { - if (isBlank(renderElementBinder.textBindings)) return []; - return ListWrapper.map(renderElementBinder.textBindings, b => BindingRecord.createForTextNode(b, this._textNodeIndex++)); - } + _createTextNodeRecords(bindings: List, + renderElementBinder: renderApi.ElementBinder) { + if (isBlank(renderElementBinder.textBindings)) return; - _createElementPropertyRecords(boundElementIndex:number, renderElementBinder:renderApi.ElementBinder) { - var res = []; - MapWrapper.forEach(renderElementBinder.propertyBindings, (astWithSource, propertyName) => { - ListWrapper.push(res, BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName)); + ListWrapper.forEach(renderElementBinder.textBindings, (b) => { + ListWrapper.push(bindings, BindingRecord.createForTextNode(b, this._textNodeIndex++)); }); - return res; } - _createDirectiveRecords(boundElementIndex:number, directiveBinders:List, + _createElementPropertyRecords(bindings: List, + boundElementIndex:number, renderElementBinder:renderApi.ElementBinder) { + MapWrapper.forEach(renderElementBinder.propertyBindings, (astWithSource, propertyName) => { + ListWrapper.push(bindings, + BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName)); + }); + } + + _createDirectiveRecords(bindings: List, + boundElementIndex:number, directiveBinders:List, allDirectiveMetadatas:List) { - var res = []; for (var i = 0; i < directiveBinders.length; i++) { var directiveBinder = directiveBinders[i]; var directiveMetadata = allDirectiveMetadatas[directiveBinder.directiveIndex]; @@ -79,18 +85,17 @@ class BindingRecordsCreator { // it monomorphic! var setter = reflector.setter(propertyName); var directiveRecord = this._getDirectiveRecord(boundElementIndex, i, directiveMetadata); - var b = BindingRecord.createForDirective(astWithSource, propertyName, setter, directiveRecord); - ListWrapper.push(res, b); + ListWrapper.push(bindings, + BindingRecord.createForDirective(astWithSource, propertyName, setter, directiveRecord)); }); // host properties MapWrapper.forEach(directiveBinder.hostPropertyBindings, (astWithSource, propertyName) => { var dirIndex = new DirectiveIndex(boundElementIndex, i); - var b = BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName); - ListWrapper.push(res, b); + ListWrapper.push(bindings, + BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName)); }); } - return res; } _getDirectiveRecord(boundElementIndex:number, directiveIndex:number, directiveMetadata:renderApi.DirectiveMetadata): DirectiveRecord {