refactor(compiler): speed up proto view merging

- Don't create intermediate merge results - Only merge embedded ProtoViews that contain `<ng-content>` tags Closes #3150 Closes #3177
2015-07-20 09:59:44 -07:00
parent de18da2a0d
commit 078475a082
20 changed files with 359 additions and 466 deletions
--- a/modules/angular2/test/core/compiler/compiler_spec.ts
+++ b/modules/angular2/test/core/compiler/compiler_spec.ts
@ -45,35 +45,6 @@ export function main() {
        rootProtoView;
    var renderCompileRequests: any[];

-    function mergeProtoViewsRecursively(
-        protoViewRefs: List<renderApi.RenderProtoViewRef | List<any>>,
-        target: renderApi.RenderProtoViewMergeMapping[]): renderApi.RenderProtoViewRef {
-      var targetIndex = target.length;
-      target.push(null);
-
-      var flattended = protoViewRefs.map(protoViewRefOrArray => {
-        var resolvedProtoViewRef;
-        if (isArray(protoViewRefOrArray)) {
-          resolvedProtoViewRef = mergeProtoViewsRecursively(
-              <List<renderApi.RenderProtoViewRef>>protoViewRefOrArray, target);
-        } else {
-          resolvedProtoViewRef = protoViewRefOrArray;
-        }
-        return resolvedProtoViewRef;
-      });
-      var merged = [];
-      flattended.forEach((entry) => {
-        if (entry instanceof MergedRenderProtoViewRef) {
-          entry.originals.forEach(ref => merged.push(ref));
-        } else {
-          merged.push(entry);
-        }
-      });
-      var result = new MergedRenderProtoViewRef(merged);
-      target[targetIndex] = new renderApi.RenderProtoViewMergeMapping(result, 1, [], [], []);
-      return result;
-    }
-
    function createCompiler(renderCompileResults:
                                List<renderApi.ProtoViewDto | Promise<renderApi.ProtoViewDto>>,
                            protoViewFactoryResults: List<AppProtoView>) {
@ -102,10 +73,10 @@ export function main() {
          });
      renderCompiler.spy('mergeProtoViewsRecursively')
          .andCallFake((protoViewRefs: List<renderApi.RenderProtoViewRef | List<any>>) => {
-            var result: renderApi.RenderProtoViewMergeMapping[] = [];
-            mergeProtoViewsRecursively(protoViewRefs, result);
-            return PromiseWrapper.resolve(result);
+            return PromiseWrapper.resolve(new renderApi.RenderProtoViewMergeMapping(
+                new MergedRenderProtoViewRef(protoViewRefs), 1, [], [], [], [null]));
          });
+      // TODO spy on .compile and return RenderProtoViewRef, same for compileHost
      rootProtoView = createRootProtoView(directiveResolver, MainComponent);
    });

@ -360,13 +331,12 @@ export function main() {
             createCompiler(renderPvDtos, [rootProtoView, mainProtoView, nestedProtoView]);
         compiler.compileInHost(MainComponent)
             .then((protoViewRef) => {
+               expect(originalRenderProtoViewRefs(internalProtoView(protoViewRef)))
+                   .toEqual(
+                       [rootProtoView.render, [mainProtoView.render, [nestedProtoView.render]]]);
               expect(internalProtoView(protoViewRef).elementBinders[0].nestedProtoView)
                   .toBe(mainProtoView);
-               expect(originalRenderProtoViewRefs(mainProtoView))
-                   .toEqual([renderPvDtos[0].render, renderPvDtos[1].render]);
               expect(mainProtoView.elementBinders[0].nestedProtoView).toBe(nestedProtoView);
-               expect(originalRenderProtoViewRefs(nestedProtoView))
-                   .toEqual([renderPvDtos[1].render]);
               async.done();
             });
       }));
@ -375,7 +345,8 @@ export function main() {
         tplResolver.setView(MainComponent, new viewAnn.View({template: '<div></div>'}));
         tplResolver.setView(NestedComponent, new viewAnn.View({template: '<div></div>'}));
         var viewportProtoView =
-             createProtoView([createComponentElementBinder(directiveResolver, NestedComponent)]);
+             createProtoView([createComponentElementBinder(directiveResolver, NestedComponent)],
+                             renderApi.ViewType.EMBEDDED);
         var mainProtoView = createProtoView([createViewportElementBinder(viewportProtoView)]);
         var nestedProtoView = createProtoView();
         var renderPvDtos = [
@ -391,23 +362,11 @@ export function main() {
             .then((protoViewRef) => {
               expect(internalProtoView(protoViewRef).elementBinders[0].nestedProtoView)
                   .toBe(mainProtoView);
-               expect(originalRenderProtoViewRefs(mainProtoView))
-                   .toEqual([
-                     renderPvDtos[0]
-                         .render,
-                     renderPvDtos[0].elementBinders[0].nestedProtoView.render,
-                     renderPvDtos[1].render
-                   ]);
+               expect(originalRenderProtoViewRefs(internalProtoView(protoViewRef)))
+                   .toEqual([rootProtoView.render, [mainProtoView.render, null]]);
               expect(viewportProtoView.elementBinders[0].nestedProtoView).toBe(nestedProtoView);
               expect(originalRenderProtoViewRefs(viewportProtoView))
-                   .toEqual([
-                     renderPvDtos[0]
-                         .elementBinders[0]
-                         .nestedProtoView.render,
-                     renderPvDtos[1].render
-                   ]);
-               expect(originalRenderProtoViewRefs(nestedProtoView))
-                   .toEqual([renderPvDtos[1].render]);
+                   .toEqual([viewportProtoView.render, [nestedProtoView.render]]);
               async.done();
             });
       }));
@ -520,7 +479,8 @@ export function main() {
       inject([AsyncTestCompleter], (async) => {
         tplResolver.setView(MainComponent, new viewAnn.View({template: '<div></div>'}));
         var viewportProtoView =
-             createProtoView([createComponentElementBinder(directiveResolver, MainComponent)]);
+             createProtoView([createComponentElementBinder(directiveResolver, MainComponent)],
+                             renderApi.ViewType.EMBEDDED);
         var mainProtoView = createProtoView([createViewportElementBinder(viewportProtoView)]);
         var renderPvDtos = [
           createRenderProtoView([
@ -539,20 +499,39 @@ export function main() {
                          .nestedProtoView)
                   .toBe(mainProtoView);
               // In case of a cycle, don't merge the embedded proto views into the component!
-               expect(originalRenderProtoViewRefs(mainProtoView))
-                   .toEqual([renderPvDtos[0].render, null]);
+               expect(originalRenderProtoViewRefs(internalProtoView(protoViewRef)))
+                   .toEqual([rootProtoView.render, [mainProtoView.render, null]]);
               expect(originalRenderProtoViewRefs(viewportProtoView))
-                   .toEqual([
-                     renderPvDtos[0]
-                         .elementBinders[0]
-                         .nestedProtoView.render,
-                     renderPvDtos[1].render,
-                     null
-                   ]);
+                   .toEqual([viewportProtoView.render, [mainProtoView.render, null]]);
               async.done();
             });
       }));

+    it('should throw on recursive components that are connected via an embedded ProtoView with <ng-content>',
+       inject([AsyncTestCompleter], (async) => {
+         tplResolver.setView(MainComponent, new viewAnn.View({template: '<div></div>'}));
+         var viewportProtoView =
+             createProtoView([createComponentElementBinder(directiveResolver, MainComponent)],
+                             renderApi.ViewType.EMBEDDED, true);
+         var mainProtoView = createProtoView([createViewportElementBinder(viewportProtoView)]);
+         var renderPvDtos = [
+           createRenderProtoView([
+             createRenderViewportElementBinder(createRenderProtoView(
+                 [createRenderComponentElementBinder(0)], renderApi.ViewType.EMBEDDED))
+           ]),
+           createRenderProtoView()
+         ];
+         var compiler = createCompiler(renderPvDtos, [rootProtoView, mainProtoView]);
+         PromiseWrapper.catchError(compiler.compileInHost(MainComponent), (e) => {
+           expect(() => { throw e; })
+               .toThrowError(
+                   `<ng-content> is used within the recursive path of ${stringify(MainComponent)}`);
+           async.done();
+           return null;
+         });
+       }));
+
+
    it('should create host proto views', inject([AsyncTestCompleter], (async) => {
         tplResolver.setView(MainComponent, new viewAnn.View({template: '<div></div>'}));
         var rootProtoView =
@ -581,8 +560,13 @@ function createDirectiveBinding(directiveResolver, type): DirectiveBinding {
  return DirectiveBinding.createFromType(type, annotation);
 }

-function createProtoView(elementBinders = null): AppProtoView {
-  var pv = new AppProtoView(null, null, null, new Map(), null);
+function createProtoView(elementBinders = null, type: renderApi.ViewType = null,
+                         isEmbeddedFragment: boolean = false): AppProtoView {
+  if (isBlank(type)) {
+    type = renderApi.ViewType.COMPONENT;
+  }
+  var pv = new AppProtoView(type, isEmbeddedFragment, new renderApi.RenderProtoViewRef(), null,
+                            null, new Map(), null);
  if (isBlank(elementBinders)) {
    elementBinders = [];
  }
@ -623,7 +607,8 @@ function createRenderViewportElementBinder(nestedProtoView): renderApi.ElementBi
 }

 function createRootProtoView(directiveResolver, type): AppProtoView {
-  return createProtoView([createComponentElementBinder(directiveResolver, type)]);
+  return createProtoView([createComponentElementBinder(directiveResolver, type)],
+                         renderApi.ViewType.HOST);
 }

@Component({selector: 'main-comp'})
--- a/modules/angular2/test/core/compiler/proto_view_factory_spec.ts
+++ b/modules/angular2/test/core/compiler/proto_view_factory_spec.ts
@ -182,7 +182,8 @@ function createRenderProtoView(elementBinders = null, type: renderApi.ViewType =
    elementBinders: elementBinders,
    type: type,
    variableBindings: variableBindings,
-    textBindings: []
+    textBindings: [],
+    transitiveNgContentCount: 0
  });
 }

--- a/modules/angular2/test/core/compiler/view_manager_utils_spec.ts
+++ b/modules/angular2/test/core/compiler/view_manager_utils_spec.ts
@ -289,13 +289,32 @@ function calcHostElementIndicesByViewIndex(pv: AppProtoView, elementOffset = 0,
  return target;
 }

+function countNestedProtoViews(pv: AppProtoView, target: number[] = null): number[] {
+  if (isBlank(target)) {
+    target = [];
+  }
+  target.push(null);
+  var resultIndex = target.length - 1;
+  var count = 0;
+  for (var binderIdx = 0; binderIdx < pv.elementBinders.length; binderIdx++) {
+    var binder = pv.elementBinders[binderIdx];
+    if (isPresent(binder.nestedProtoView)) {
+      var nextResultIndex = target.length;
+      countNestedProtoViews(binder.nestedProtoView, target);
+      count += target[nextResultIndex] + 1;
+    }
+  }
+  target[resultIndex] = count;
+  return target;
+}
+
 function _createProtoView(type: ViewType, binders: ElementBinder[] = null) {
  if (isBlank(binders)) {
    binders = [];
  }
  var protoChangeDetector = <any>new SpyProtoChangeDetector();
  protoChangeDetector.spy('instantiate').andReturn(new SpyChangeDetector());
-  var res = new AppProtoView(type, protoChangeDetector, null, null, 0);
+  var res = new AppProtoView(type, null, null, protoChangeDetector, null, null, 0);
  res.elementBinders = binders;
  var mappedElementIndices = ListWrapper.createFixedSize(countNestedElementBinders(res));
  for (var i = 0; i < binders.length; i++) {
@ -304,9 +323,11 @@ function _createProtoView(type: ViewType, binders: ElementBinder[] = null) {
    binder.protoElementInjector.index = i;
  }
  var hostElementIndicesByViewIndex = calcHostElementIndicesByViewIndex(res);
-  res.mergeMapping = new AppProtoViewMergeMapping(
-      new RenderProtoViewMergeMapping(null, hostElementIndicesByViewIndex.length,
-                                      mappedElementIndices, [], hostElementIndicesByViewIndex));
+  if (type === ViewType.EMBEDDED || type === ViewType.HOST) {
+    res.mergeMapping = new AppProtoViewMergeMapping(new RenderProtoViewMergeMapping(
+        null, hostElementIndicesByViewIndex.length, mappedElementIndices, [],
+        hostElementIndicesByViewIndex, countNestedProtoViews(res)));
+  }
  return res;
 }

--- a/modules/angular2/test/core/compiler/view_pool_spec.ts
+++ b/modules/angular2/test/core/compiler/view_pool_spec.ts
@ -24,7 +24,9 @@ export function main() {

    function createViewPool({capacity}): AppViewPool { return new AppViewPool(capacity); }

-    function createProtoView() { return new AppProtoView(null, null, null, null, null); }
+    function createProtoView() {
+      return new AppProtoView(null, null, null, null, null, null, null);
+    }

    function createView(pv) {
      return new AppView(null, pv, null, null, null, null, new Map(), null, null);