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 `<content>`.

Closes #2297
This commit is contained in:
Tobias Bosch 2015-06-02 14:14:10 -07:00
parent c2fa4b7191
commit ba7956f521
5 changed files with 49 additions and 6 deletions

View File

@ -52,6 +52,13 @@ export class LightDom {
// Collects the Content directives from the view and all its child views // Collects the Content directives from the view and all its child views
private _collectAllContentTags(view: viewModule.DomView, acc: List<Content>): List<Content> { private _collectAllContentTags(view: viewModule.DomView, acc: List<Content>): List<Content> {
// 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 contentTags = view.contentTags;
var vcs = view.viewContainers; var vcs = view.viewContainers;
for (var i = 0; i < vcs.length; i++) { for (var i = 0; i < vcs.length; i++) {

View File

@ -25,10 +25,13 @@ export class DomProtoView {
elementBinders: List<ElementBinder>; elementBinders: List<ElementBinder>;
isTemplateElement: boolean; isTemplateElement: boolean;
rootBindingOffset: number; 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.element = element;
this.elementBinders = elementBinders; this.elementBinders = elementBinders;
this.transitiveContentTagCount = transitiveContentTagCount;
this.isTemplateElement = DOM.isTemplateElement(this.element); this.isTemplateElement = DOM.isTemplateElement(this.element);
this.rootBindingOffset = this.rootBindingOffset =
(isPresent(this.element) && DOM.hasClass(this.element, NG_BINDING_CLASS)) ? 1 : 0; (isPresent(this.element) && DOM.hasClass(this.element, NG_BINDING_CLASS)) ? 1 : 0;

View File

@ -54,6 +54,7 @@ export class ProtoViewBuilder {
var renderElementBinders = []; var renderElementBinders = [];
var apiElementBinders = []; var apiElementBinders = [];
var transitiveContentTagCount = 0;
ListWrapper.forEach(this.elements, (ebb) => { ListWrapper.forEach(this.elements, (ebb) => {
var propertySetters = MapWrapper.create(); var propertySetters = MapWrapper.create();
var hostActions = MapWrapper.create(); var hostActions = MapWrapper.create();
@ -82,6 +83,14 @@ export class ProtoViewBuilder {
}); });
var nestedProtoView = isPresent(ebb.nestedProtoView) ? ebb.nestedProtoView.build() : null; 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; var parentIndex = isPresent(ebb.parent) ? ebb.parent.index : -1;
ListWrapper.push(apiElementBinders, new api.ElementBinder({ ListWrapper.push(apiElementBinders, new api.ElementBinder({
index: ebb.index, index: ebb.index,
@ -112,8 +121,11 @@ export class ProtoViewBuilder {
})); }));
}); });
return new api.ProtoViewDto({ return new api.ProtoViewDto({
render: new DomProtoViewRef( render: new DomProtoViewRef(new DomProtoView({
new DomProtoView({element: this.rootElement, elementBinders: renderElementBinders})), element: this.rootElement,
elementBinders: renderElementBinders,
transitiveContentTagCount: transitiveContentTagCount
})),
type: this.type, type: this.type,
elementBinders: apiElementBinders, elementBinders: apiElementBinders,
variableBindings: this.variableBindings variableBindings: this.variableBindings

View File

@ -16,17 +16,28 @@ import {DOM} from 'angular2/src/dom/dom_adapter';
import {Content} from 'angular2/src/render/dom/shadow_dom/content_tag'; import {Content} from 'angular2/src/render/dom/shadow_dom/content_tag';
import {LightDom} from 'angular2/src/render/dom/shadow_dom/light_dom'; import {LightDom} from 'angular2/src/render/dom/shadow_dom/light_dom';
import {DomView} from 'angular2/src/render/dom/view/view'; 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'; 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 @proxy
@IMPLEMENTS(DomView) @IMPLEMENTS(DomView)
class FakeView extends SpyObject { class FakeView extends SpyObject {
boundElements; boundElements;
contentTags; contentTags;
viewContainers; viewContainers;
proto;
constructor(containers = null) { constructor(containers = null, transitiveContentTagCount: number = 1) {
super(DomView); super(DomView);
this.proto = new FakeProtoView(transitiveContentTagCount);
this.boundElements = []; this.boundElements = [];
this.contentTags = []; this.contentTags = [];
this.viewContainers = []; this.viewContainers = [];
@ -109,7 +120,7 @@ export function main() {
beforeEach(() => { lightDomView = new FakeView(); }); beforeEach(() => { lightDomView = new FakeView(); });
describe("contentTags", () => { describe("contentTags", () => {
it("should collect content tags from element injectors", () => { it("should collect unconditional content tags", () => {
var tag = new FakeContentTag(el('<script></script>')); var tag = new FakeContentTag(el('<script></script>'));
var shadowDomView = new FakeView([tag]); var shadowDomView = new FakeView([tag]);
@ -126,6 +137,15 @@ export function main() {
expect(lightDom.contentTags()).toEqual([tag]); expect(lightDom.contentTags()).toEqual([tag]);
}); });
it("should not walk views that can't have content tags", () => {
var tag = new FakeContentTag(el('<script></script>'));
var shadowDomView = new FakeView([tag], 0);
var lightDom = createLightDom(lightDomView, shadowDomView, el("<div></div>"));
expect(lightDom.contentTags()).toEqual([]);
});
}); });
describe("expandedDomNodes", () => { describe("expandedDomNodes", () => {

View File

@ -31,7 +31,8 @@ export function main() {
binders = []; binders = [];
} }
var rootEl = el('<div></div>'); var rootEl = el('<div></div>');
return new DomProtoView({element: rootEl, elementBinders: binders}); return new DomProtoView(
{element: rootEl, elementBinders: binders, transitiveContentTagCount: 0});
} }
function createView(pv = null, boundElementCount = 0) { function createView(pv = null, boundElementCount = 0) {