From ba7956f52187940ba901980858e09976505787b5 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Tue, 2 Jun 2015 14:14:10 -0700 Subject: [PATCH] fix(render): only look for content tags in views that might have them. Largetable benchmark with `interpolationAttr` and 200 rows / 20 columns: Time for destroy/create pair dropped from about 1260ms to about 150ms. Related to #2298, but does not really fix it as we are still slow if people are using ``. Closes #2297 --- .../src/render/dom/shadow_dom/light_dom.ts | 7 ++++++ .../src/render/dom/view/proto_view.ts | 5 +++- .../src/render/dom/view/proto_view_builder.ts | 16 +++++++++++-- .../render/dom/shadow_dom/light_dom_spec.ts | 24 +++++++++++++++++-- .../test/render/dom/view/view_spec.ts | 3 ++- 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/modules/angular2/src/render/dom/shadow_dom/light_dom.ts b/modules/angular2/src/render/dom/shadow_dom/light_dom.ts index 8b3caa22ce..76296ff682 100644 --- a/modules/angular2/src/render/dom/shadow_dom/light_dom.ts +++ b/modules/angular2/src/render/dom/shadow_dom/light_dom.ts @@ -52,6 +52,13 @@ export class LightDom { // Collects the Content directives from the view and all its child views private _collectAllContentTags(view: viewModule.DomView, acc: List): List { + // Note: exiting early here is important as we call this function for every view + // that is added, so we have O(n^2) runtime. + // TODO(tbosch): fix the root problem, see + // https://github.com/angular/angular/issues/2298 + if (view.proto.transitiveContentTagCount === 0) { + return acc; + } var contentTags = view.contentTags; var vcs = view.viewContainers; for (var i = 0; i < vcs.length; i++) { diff --git a/modules/angular2/src/render/dom/view/proto_view.ts b/modules/angular2/src/render/dom/view/proto_view.ts index 74bb93b72b..631d5a6afe 100644 --- a/modules/angular2/src/render/dom/view/proto_view.ts +++ b/modules/angular2/src/render/dom/view/proto_view.ts @@ -25,10 +25,13 @@ export class DomProtoView { elementBinders: List; isTemplateElement: boolean; rootBindingOffset: number; + // the number of content tags seen in this or any child proto view. + transitiveContentTagCount: number; - constructor({elementBinders, element}) { + constructor({elementBinders, element, transitiveContentTagCount}) { this.element = element; this.elementBinders = elementBinders; + this.transitiveContentTagCount = transitiveContentTagCount; this.isTemplateElement = DOM.isTemplateElement(this.element); this.rootBindingOffset = (isPresent(this.element) && DOM.hasClass(this.element, NG_BINDING_CLASS)) ? 1 : 0; diff --git a/modules/angular2/src/render/dom/view/proto_view_builder.ts b/modules/angular2/src/render/dom/view/proto_view_builder.ts index 26a65ada46..9e0e4e4cdc 100644 --- a/modules/angular2/src/render/dom/view/proto_view_builder.ts +++ b/modules/angular2/src/render/dom/view/proto_view_builder.ts @@ -54,6 +54,7 @@ export class ProtoViewBuilder { var renderElementBinders = []; var apiElementBinders = []; + var transitiveContentTagCount = 0; ListWrapper.forEach(this.elements, (ebb) => { var propertySetters = MapWrapper.create(); var hostActions = MapWrapper.create(); @@ -82,6 +83,14 @@ export class ProtoViewBuilder { }); var nestedProtoView = isPresent(ebb.nestedProtoView) ? ebb.nestedProtoView.build() : null; + var nestedRenderProtoView = + isPresent(nestedProtoView) ? resolveInternalDomProtoView(nestedProtoView.render) : null; + if (isPresent(nestedRenderProtoView)) { + transitiveContentTagCount += nestedRenderProtoView.transitiveContentTagCount; + } + if (isPresent(ebb.contentTagSelector)) { + transitiveContentTagCount++; + } var parentIndex = isPresent(ebb.parent) ? ebb.parent.index : -1; ListWrapper.push(apiElementBinders, new api.ElementBinder({ index: ebb.index, @@ -112,8 +121,11 @@ export class ProtoViewBuilder { })); }); return new api.ProtoViewDto({ - render: new DomProtoViewRef( - new DomProtoView({element: this.rootElement, elementBinders: renderElementBinders})), + render: new DomProtoViewRef(new DomProtoView({ + element: this.rootElement, + elementBinders: renderElementBinders, + transitiveContentTagCount: transitiveContentTagCount + })), type: this.type, elementBinders: apiElementBinders, variableBindings: this.variableBindings diff --git a/modules/angular2/test/render/dom/shadow_dom/light_dom_spec.ts b/modules/angular2/test/render/dom/shadow_dom/light_dom_spec.ts index cc68d77207..d6db328ec5 100644 --- a/modules/angular2/test/render/dom/shadow_dom/light_dom_spec.ts +++ b/modules/angular2/test/render/dom/shadow_dom/light_dom_spec.ts @@ -16,17 +16,28 @@ import {DOM} from 'angular2/src/dom/dom_adapter'; import {Content} from 'angular2/src/render/dom/shadow_dom/content_tag'; import {LightDom} from 'angular2/src/render/dom/shadow_dom/light_dom'; import {DomView} from 'angular2/src/render/dom/view/view'; +import {DomProtoView} from 'angular2/src/render/dom/view/proto_view'; import {DomViewContainer} from 'angular2/src/render/dom/view/view_container'; +@proxy +@IMPLEMENTS(DomProtoView) +class FakeProtoView extends SpyObject { + constructor(public transitiveContentTagCount: number) { super(DomProtoView); } + + noSuchMethod(i) { super.noSuchMethod(i); } +} + @proxy @IMPLEMENTS(DomView) class FakeView extends SpyObject { boundElements; contentTags; viewContainers; + proto; - constructor(containers = null) { + constructor(containers = null, transitiveContentTagCount: number = 1) { super(DomView); + this.proto = new FakeProtoView(transitiveContentTagCount); this.boundElements = []; this.contentTags = []; this.viewContainers = []; @@ -109,7 +120,7 @@ export function main() { beforeEach(() => { lightDomView = new FakeView(); }); describe("contentTags", () => { - it("should collect content tags from element injectors", () => { + it("should collect unconditional content tags", () => { var tag = new FakeContentTag(el('')); var shadowDomView = new FakeView([tag]); @@ -126,6 +137,15 @@ export function main() { expect(lightDom.contentTags()).toEqual([tag]); }); + + it("should not walk views that can't have content tags", () => { + var tag = new FakeContentTag(el('')); + var shadowDomView = new FakeView([tag], 0); + + var lightDom = createLightDom(lightDomView, shadowDomView, el("
")); + + expect(lightDom.contentTags()).toEqual([]); + }); }); describe("expandedDomNodes", () => { diff --git a/modules/angular2/test/render/dom/view/view_spec.ts b/modules/angular2/test/render/dom/view/view_spec.ts index fb37e392cf..e96f97c6ba 100644 --- a/modules/angular2/test/render/dom/view/view_spec.ts +++ b/modules/angular2/test/render/dom/view/view_spec.ts @@ -31,7 +31,8 @@ export function main() { binders = []; } var rootEl = el('
'); - return new DomProtoView({element: rootEl, elementBinders: binders}); + return new DomProtoView( + {element: rootEl, elementBinders: binders, transitiveContentTagCount: 0}); } function createView(pv = null, boundElementCount = 0) {