Skip to content

Commit 078475a

Browse files
tboschtbosch
committed
refactor(compiler): speed up proto view merging
- Don't create intermediate merge results - Only merge embedded ProtoViews that contain `<ng-content>` tags Closes angular#3150 Closes angular#3177
1 parent de18da2 commit 078475a

20 files changed

Lines changed: 358 additions & 465 deletions

‎modules/angular2/src/core/compiler/compiler.ts‎

Lines changed: 65 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {ComponentUrlMapper} from './component_url_mapper';
2424
import {ProtoViewFactory} from './proto_view_factory';
2525
import {UrlResolver} from 'angular2/src/services/url_resolver';
2626
import {AppRootUrl} from 'angular2/src/services/app_root_url';
27+
import {ElementBinder} from './element_binder';
2728

2829
import * as renderApi from 'angular2/src/render/api';
2930

@@ -90,7 +91,7 @@ export class Compiler {
9091
private _appUrl: string;
9192
private _render: renderApi.RenderCompiler;
9293
private _protoViewFactory: ProtoViewFactory;
93-
private _unmergedCyclicEmbeddedProtoViews: RecursiveEmbeddedProtoView[] = [];
94+
private _protoViewsToBeMerged: AppProtoView[] = [];
9495

9596
/**
9697
* @private
@@ -146,8 +147,37 @@ export class Compiler {
146147
return this._compileNestedProtoViews(hostRenderPv, protoView, componentType);
147148
});
148149
}
149-
return hostPvPromise.then(
150-
hostAppProtoView => this._mergeCyclicEmbeddedProtoViews().then(_ => hostAppProtoView.ref));
150+
return hostPvPromise.then(hostAppProtoView =>
151+
this._mergeUnmergedProtoViews().then(_ => hostAppProtoView.ref));
152+
}
153+
154+
private _mergeUnmergedProtoViews(): Promise<any> {
155+
var protoViewsToBeMerged = this._protoViewsToBeMerged;
156+
this._protoViewsToBeMerged = [];
157+
return PromiseWrapper.all(protoViewsToBeMerged.map((appProtoView) => {
158+
return this._render.mergeProtoViewsRecursively(
159+
this._collectMergeRenderProtoViews(appProtoView))
160+
.then((mergeResult: renderApi.RenderProtoViewMergeMapping) => {
161+
appProtoView.mergeMapping = new AppProtoViewMergeMapping(mergeResult);
162+
});
163+
}));
164+
}
165+
166+
private _collectMergeRenderProtoViews(
167+
appProtoView: AppProtoView): List<renderApi.RenderProtoViewRef | List<any>> {
168+
var result = [appProtoView.render];
169+
for (var i = 0; i < appProtoView.elementBinders.length; i++) {
170+
var binder = appProtoView.elementBinders[i];
171+
if (isPresent(binder.nestedProtoView)) {
172+
if (binder.hasStaticComponent() ||
173+
(binder.hasEmbeddedProtoView() && binder.nestedProtoView.isEmbeddedFragment)) {
174+
result.push(this._collectMergeRenderProtoViews(binder.nestedProtoView));
175+
} else {
176+
result.push(null);
177+
}
178+
}
179+
}
180+
return result;
151181
}
152182

153183
private _compile(componentBinding: DirectiveBinding): Promise<AppProtoView>| AppProtoView {
@@ -207,7 +237,7 @@ export class Compiler {
207237
appProtoView: AppProtoView,
208238
componentType: Type): Promise<AppProtoView> {
209239
var nestedPVPromises = [];
210-
this._loopComponentElementBinders(appProtoView, (parentPv, elementBinder) => {
240+
this._loopComponentElementBinders(appProtoView, (parentPv, elementBinder: ElementBinder) => {
211241
var nestedComponent = elementBinder.componentDirective;
212242
var elementBinderDone =
213243
(nestedPv: AppProtoView) => { elementBinder.nestedProtoView = nestedPv; };
@@ -220,83 +250,50 @@ export class Compiler {
220250
});
221251
return PromiseWrapper.all(nestedPVPromises)
222252
.then((_) => {
223-
var appProtoViewsToMergeInto = [];
224-
var mergeRenderProtoViews = this._collectMergeRenderProtoViewsRecurse(
225-
renderProtoView, appProtoView, appProtoViewsToMergeInto);
226-
if (isBlank(mergeRenderProtoViews)) {
227-
throw new BaseException(`Unconditional component cycle in ${stringify(componentType)}`);
228-
}
229-
return this._mergeProtoViews(appProtoViewsToMergeInto, mergeRenderProtoViews);
253+
this._collectMergableProtoViews(appProtoView, componentType);
254+
return appProtoView;
230255
});
231256
}
232257

233-
private _mergeProtoViews(
234-
appProtoViewsToMergeInto: AppProtoView[],
235-
mergeRenderProtoViews:
236-
List<renderApi.RenderProtoViewRef | List<any>>): Promise<AppProtoView> {
237-
return this._render.mergeProtoViewsRecursively(mergeRenderProtoViews)
238-
.then((mergeResults: List<renderApi.RenderProtoViewMergeMapping>) => {
239-
// Note: We don't need to check for nulls here as we filtered them out before!
240-
// (in RenderCompiler.mergeProtoViewsRecursively and
241-
// _collectMergeRenderProtoViewsRecurse).
242-
for (var i = 0; i < mergeResults.length; i++) {
243-
appProtoViewsToMergeInto[i].mergeMapping =
244-
new AppProtoViewMergeMapping(mergeResults[i]);
245-
}
246-
return appProtoViewsToMergeInto[0];
247-
});
248-
}
249-
250-
private _loopComponentElementBinders(appProtoView: AppProtoView, callback: Function) {
251-
appProtoView.elementBinders.forEach((elementBinder) => {
252-
if (isPresent(elementBinder.componentDirective)) {
253-
callback(appProtoView, elementBinder);
254-
} else if (isPresent(elementBinder.nestedProtoView)) {
255-
this._loopComponentElementBinders(elementBinder.nestedProtoView, callback);
256-
}
257-
});
258-
}
259-
260-
private _collectMergeRenderProtoViewsRecurse(
261-
renderProtoView: renderApi.ProtoViewDto, appProtoView: AppProtoView,
262-
targetAppProtoViews: AppProtoView[]): List<renderApi.RenderProtoViewRef | List<any>> {
263-
targetAppProtoViews.push(appProtoView);
264-
var result = [renderProtoView.render];
258+
private _collectMergableProtoViews(appProtoView: AppProtoView, componentType: Type) {
259+
var isRecursive = false;
265260
for (var i = 0; i < appProtoView.elementBinders.length; i++) {
266261
var binder = appProtoView.elementBinders[i];
267262
if (binder.hasStaticComponent()) {
268-
if (isBlank(binder.nestedProtoView.mergeMapping)) {
269-
// cycle via an embedded ProtoView. store the AppProtoView and ProtoViewDto for later.
270-
this._unmergedCyclicEmbeddedProtoViews.push(
271-
new RecursiveEmbeddedProtoView(appProtoView, renderProtoView));
272-
return null;
263+
if (isBlank(binder.nestedProtoView.isRecursive)) {
264+
// cycle via a component. We are in the tail recursion,
265+
// so all components should have their isRecursive flag set already.
266+
isRecursive = true;
267+
break;
273268
}
274-
result.push(binder.nestedProtoView.mergeMapping.renderProtoViewRef);
275269
} else if (binder.hasEmbeddedProtoView()) {
276-
result.push(this._collectMergeRenderProtoViewsRecurse(
277-
renderProtoView.elementBinders[i].nestedProtoView, binder.nestedProtoView,
278-
targetAppProtoViews));
270+
this._collectMergableProtoViews(binder.nestedProtoView, componentType);
279271
}
280272
}
281-
return result;
273+
if (isRecursive) {
274+
if (appProtoView.isEmbeddedFragment) {
275+
throw new BaseException(
276+
`<ng-content> is used within the recursive path of ${stringify(componentType)}`);
277+
}
278+
if (appProtoView.type === renderApi.ViewType.COMPONENT) {
279+
throw new BaseException(`Unconditional component cycle in ${stringify(componentType)}`);
280+
}
281+
}
282+
if (appProtoView.type === renderApi.ViewType.EMBEDDED ||
283+
appProtoView.type === renderApi.ViewType.HOST) {
284+
this._protoViewsToBeMerged.push(appProtoView);
285+
}
286+
appProtoView.isRecursive = isRecursive;
282287
}
283288

284-
private _mergeCyclicEmbeddedProtoViews() {
285-
var pvs = this._unmergedCyclicEmbeddedProtoViews;
286-
this._unmergedCyclicEmbeddedProtoViews = [];
287-
var promises = pvs.map(entry => {
288-
var appProtoView = entry.appProtoView;
289-
var mergeRenderProtoViews = [entry.renderProtoView.render];
290-
appProtoView.elementBinders.forEach((binder) => {
291-
if (binder.hasStaticComponent()) {
292-
mergeRenderProtoViews.push(binder.nestedProtoView.mergeMapping.renderProtoViewRef);
293-
} else if (binder.hasEmbeddedProtoView()) {
294-
mergeRenderProtoViews.push(null);
295-
}
296-
});
297-
return this._mergeProtoViews([appProtoView], mergeRenderProtoViews);
289+
private _loopComponentElementBinders(appProtoView: AppProtoView, callback: Function) {
290+
appProtoView.elementBinders.forEach((elementBinder) => {
291+
if (isPresent(elementBinder.componentDirective)) {
292+
callback(appProtoView, elementBinder);
293+
} else if (isPresent(elementBinder.nestedProtoView)) {
294+
this._loopComponentElementBinders(elementBinder.nestedProtoView, callback);
295+
}
298296
});
299-
return PromiseWrapper.all(promises);
300297
}
301298

302299
private _buildRenderTemplate(component, view, directives): renderApi.ViewDefinition {
@@ -356,7 +353,3 @@ export class Compiler {
356353
}
357354
}
358355
}
359-
360-
class RecursiveEmbeddedProtoView {
361-
constructor(public appProtoView: AppProtoView, public renderProtoView: renderApi.ProtoViewDto) {}
362-
}

‎modules/angular2/src/core/compiler/proto_view_factory.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,12 @@ function _createAppProtoView(
254254
renderProtoView: renderApi.ProtoViewDto, protoChangeDetector: ProtoChangeDetector,
255255
variableBindings: Map<string, string>, allDirectives: List<DirectiveBinding>): AppProtoView {
256256
var elementBinders = renderProtoView.elementBinders;
257-
var protoView = new AppProtoView(renderProtoView.type, protoChangeDetector, variableBindings,
258-
createVariableLocations(elementBinders),
259-
renderProtoView.textBindings.length);
257+
// Embedded ProtoViews that contain `<ng-content>` will be merged into their parents and use
258+
// a RenderFragmentRef. I.e. renderProtoView.transitiveNgContentCount > 0.
259+
var protoView = new AppProtoView(
260+
renderProtoView.type, renderProtoView.transitiveNgContentCount > 0, renderProtoView.render,
261+
protoChangeDetector, variableBindings, createVariableLocations(elementBinders),
262+
renderProtoView.textBindings.length);
260263
_createElementBinders(protoView, elementBinders, allDirectives);
261264
_bindDirectiveEvents(protoView, elementBinders);
262265

‎modules/angular2/src/core/compiler/view.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export class AppProtoViewMergeMapping {
3232
renderTextIndices: number[];
3333
nestedViewIndicesByElementIndex: number[];
3434
hostElementIndicesByViewIndex: number[];
35+
nestedViewCountByViewIndex: number[];
3536
constructor(renderProtoViewMergeMapping: renderApi.RenderProtoViewMergeMapping) {
3637
this.renderProtoViewRef = renderProtoViewMergeMapping.mergedProtoViewRef;
3738
this.renderFragmentCount = renderProtoViewMergeMapping.fragmentCount;
@@ -42,11 +43,8 @@ export class AppProtoViewMergeMapping {
4243
this.hostElementIndicesByViewIndex = renderProtoViewMergeMapping.hostElementIndicesByViewIndex;
4344
this.nestedViewIndicesByElementIndex =
4445
inverseIndexMapping(this.hostElementIndicesByViewIndex, this.renderElementIndices.length);
46+
this.nestedViewCountByViewIndex = renderProtoViewMergeMapping.nestedViewCountByViewIndex;
4547
}
46-
47-
get viewCount() { return this.hostElementIndicesByViewIndex.length; }
48-
49-
get elementCount() { return this.renderElementIndices.length; }
5048
}
5149

5250
function inverseIndexMapping(input: number[], resultLength: number): number[] {
@@ -215,7 +213,7 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
215213
dispatchRenderEvent(renderElementIndex: number, eventName: string,
216214
locals: Map<string, any>): boolean {
217215
var elementRef =
218-
this.elementRefs[this.proto.mergeMapping.renderInverseElementIndices[renderElementIndex]];
216+
this.elementRefs[this.mainMergeMapping.renderInverseElementIndices[renderElementIndex]];
219217
var view = internalView(elementRef.parentView);
220218
return view.dispatchEvent(elementRef.boundElementIndex, eventName, locals);
221219
}
@@ -258,7 +256,11 @@ export class AppProtoView {
258256
mergeMapping: AppProtoViewMergeMapping;
259257
ref: ProtoViewRef;
260258

261-
constructor(public type: renderApi.ViewType, public protoChangeDetector: ProtoChangeDetector,
259+
isRecursive: boolean = null;
260+
261+
constructor(public type: renderApi.ViewType, public isEmbeddedFragment: boolean,
262+
public render: renderApi.RenderProtoViewRef,
263+
public protoChangeDetector: ProtoChangeDetector,
262264
public variableBindings: Map<string, string>,
263265
public variableLocations: Map<string, number>, public textBindingCount: number) {
264266
this.ref = new ProtoViewRef(this);

‎modules/angular2/src/core/compiler/view_manager.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,19 @@ export class AppViewManager {
339339
this._utils.dehydrateView(view);
340340
}
341341
var viewContainers = view.viewContainers;
342-
for (var i = view.elementOffset, ii = view.elementOffset + view.proto.mergeMapping.elementCount;
343-
i < ii; i++) {
344-
var vc = viewContainers[i];
345-
if (isPresent(vc)) {
346-
for (var j = vc.views.length - 1; j >= 0; j--) {
347-
this._destroyViewInContainer(view, i, j);
342+
var startViewOffset = view.viewOffset;
343+
var endViewOffset =
344+
view.viewOffset + view.mainMergeMapping.nestedViewCountByViewIndex[view.viewOffset];
345+
var elementOffset = view.elementOffset;
346+
for (var viewIdx = startViewOffset; viewIdx <= endViewOffset; viewIdx++) {
347+
var currView = view.views[viewIdx];
348+
for (var binderIdx = 0; binderIdx < currView.proto.elementBinders.length;
349+
binderIdx++, elementOffset++) {
350+
var vc = viewContainers[elementOffset];
351+
if (isPresent(vc)) {
352+
for (var j = vc.views.length - 1; j >= 0; j--) {
353+
this._destroyViewInContainer(currView, elementOffset, j);
354+
}
348355
}
349356
}
350357
}

‎modules/angular2/src/core/compiler/view_manager_utils.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ export class AppViewManagerUtils {
2626
var renderFragments = renderViewWithFragments.fragmentRefs;
2727
var renderView = renderViewWithFragments.viewRef;
2828

29-
var elementCount = mergedParentViewProto.mergeMapping.elementCount;
30-
var viewCount = mergedParentViewProto.mergeMapping.viewCount;
29+
var elementCount = mergedParentViewProto.mergeMapping.renderElementIndices.length;
30+
var viewCount = mergedParentViewProto.mergeMapping.nestedViewCountByViewIndex[0] + 1;
3131
var elementRefs: ElementRef[] = ListWrapper.createFixedSize(elementCount);
3232
var viewContainers = ListWrapper.createFixedSize(elementCount);
3333
var preBuiltObjects: eli.PreBuiltObjects[] = ListWrapper.createFixedSize(elementCount);
@@ -175,13 +175,13 @@ export class AppViewManagerUtils {
175175
_hydrateView(initView: viewModule.AppView, imperativelyCreatedInjector: Injector,
176176
hostElementInjector: eli.ElementInjector, context: Object, parentLocals: Locals) {
177177
var viewIdx = initView.viewOffset;
178-
var endViewOffset = viewIdx + initView.proto.mergeMapping.viewCount;
179-
while (viewIdx < endViewOffset) {
178+
var endViewOffset = viewIdx + initView.mainMergeMapping.nestedViewCountByViewIndex[viewIdx];
179+
while (viewIdx <= endViewOffset) {
180180
var currView = initView.views[viewIdx];
181181
var currProtoView = currView.proto;
182182
if (currView !== initView && currView.proto.type === ViewType.EMBEDDED) {
183183
// Don't hydrate components of embedded fragment views.
184-
viewIdx += currProtoView.mergeMapping.viewCount;
184+
viewIdx += initView.mainMergeMapping.nestedViewCountByViewIndex[viewIdx] + 1;
185185
} else {
186186
if (currView !== initView) {
187187
// hydrate a nested component view
@@ -263,9 +263,9 @@ export class AppViewManagerUtils {
263263
}
264264

265265
dehydrateView(initView: viewModule.AppView) {
266-
for (var viewIdx = initView.viewOffset,
267-
endViewOffset = viewIdx + initView.proto.mergeMapping.viewCount;
268-
viewIdx < endViewOffset; viewIdx++) {
266+
var endViewOffset = initView.viewOffset +
267+
initView.mainMergeMapping.nestedViewCountByViewIndex[initView.viewOffset];
268+
for (var viewIdx = initView.viewOffset; viewIdx <= endViewOffset; viewIdx++) {
269269
var currView = initView.views[viewIdx];
270270
if (currView.hydrated()) {
271271
if (isPresent(currView.locals)) {

‎modules/angular2/src/render/api.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,23 @@ export class ProtoViewDto {
114114
variableBindings: Map<string, string>;
115115
type: ViewType;
116116
textBindings: List<ASTWithSource>;
117+
transitiveNgContentCount: number;
117118

118-
constructor({render, elementBinders, variableBindings, type, textBindings}: {
119+
constructor({render, elementBinders, variableBindings, type, textBindings,
120+
transitiveNgContentCount}: {
119121
render?: RenderProtoViewRef,
120122
elementBinders?: List<ElementBinder>,
121123
variableBindings?: Map<string, string>,
122124
type?: ViewType,
123-
textBindings?: List<ASTWithSource>
125+
textBindings?: List<ASTWithSource>,
126+
transitiveNgContentCount?: number
124127
}) {
125128
this.render = render;
126129
this.elementBinders = elementBinders;
127130
this.variableBindings = variableBindings;
128131
this.type = type;
129132
this.textBindings = textBindings;
133+
this.transitiveNgContentCount = transitiveNgContentCount;
130134
}
131135
}
132136

@@ -308,7 +312,9 @@ export class RenderProtoViewMergeMapping {
308312
// indices for one ProtoView in a consecuitve block.
309313
public mappedTextIndices: number[],
310314
// Mapping from view index to app element index
311-
public hostElementIndicesByViewIndex: number[]) {}
315+
public hostElementIndicesByViewIndex: number[],
316+
// Number of contained views by view index
317+
public nestedViewCountByViewIndex: number[]) {}
312318
}
313319

314320
export class RenderCompiler {
@@ -331,10 +337,10 @@ export class RenderCompiler {
331337
* If the array contains other arrays, they will be merged before processing the parent array.
332338
* The array must contain an entry for every component and embedded ProtoView of the first entry.
333339
* @param protoViewRefs List of ProtoViewRefs or nested
334-
* @return the merge result for every input array in depth first order.
340+
* @return the merge result
335341
*/
336342
mergeProtoViewsRecursively(
337-
protoViewRefs: List<RenderProtoViewRef | List<any>>): Promise<RenderProtoViewMergeMapping[]> {
343+
protoViewRefs: List<RenderProtoViewRef | List<any>>): Promise<RenderProtoViewMergeMapping> {
338344
return null;
339345
}
340346
}

‎modules/angular2/src/render/dom/compiler/compiler.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ export class DomCompiler extends RenderCompiler {
5454
}
5555

5656
mergeProtoViewsRecursively(
57-
protoViewRefs:
58-
List<RenderProtoViewRef | List<any>>): Promise<List<RenderProtoViewMergeMapping>> {
57+
protoViewRefs: List<RenderProtoViewRef | List<any>>): Promise<RenderProtoViewMergeMapping> {
5958
return PromiseWrapper.resolve(pvm.mergeProtoViewsRecursively(protoViewRefs));
6059
}
6160

‎modules/angular2/src/render/dom/shadow_dom/shadow_dom_compile_step.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
import {DOM} from 'angular2/src/dom/dom_adapter';
2-
31
import {CompileStep} from '../compiler/compile_step';
42
import {CompileElement} from '../compiler/compile_element';
53
import {CompileControl} from '../compiler/compile_control';
64
import {ViewDefinition} from '../../api';
75
import {ShadowDomStrategy} from './shadow_dom_strategy';
6+
import {NG_CONTENT_ELEMENT_NAME, isElementWithTag} from '../util';
87

98
export class ShadowDomCompileStep implements CompileStep {
109
constructor(public _shadowDomStrategy: ShadowDomStrategy, public _view: ViewDefinition) {}
1110

1211
process(parent: CompileElement, current: CompileElement, control: CompileControl) {
13-
var tagName = DOM.tagName(current.element).toUpperCase();
14-
if (tagName == 'STYLE') {
12+
if (isElementWithTag(current.element, NG_CONTENT_ELEMENT_NAME)) {
13+
current.inheritedProtoView.bindNgContent();
14+
} else if (isElementWithTag(current.element, 'style')) {
1515
this._processStyleElement(current, control);
1616
} else {
1717
var componentId = current.isBound() ? current.inheritedElementBinder.componentId : null;

0 commit comments

Comments
 (0)