refactor(ivy): remove ngPrivateData megamorphic prop access (angular#30548)

PR Close angular#30548

Author: mhevery

packages/core/src/render3/component.ts

@@ -174,11 +174,8 @@ export function createRootComponentView(
   const tView = rootView[TVIEW];
   const tNode: TElementNode = createNodeAtIndex(0, TNodeType.Element, rNode, null, null);
   const componentView = createLView(
-      rootView, getOrCreateTView(
-                    def.template, def.consts, def.vars, def.directiveDefs, def.pipeDefs,
-                    def.viewQuery, def.schemas),
-      null, def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways, rootView[HEADER_OFFSET], tNode,
-      rendererFactory, renderer, sanitizer);
+      rootView, getOrCreateTView(def), null, def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways,
+      rootView[HEADER_OFFSET], tNode, rendererFactory, renderer, sanitizer);
 
   if (tView.firstTemplatePass) {
     diPublicInInjector(getOrCreateNodeInjectorForNode(tNode, rootView), rootView, def.type);

packages/core/src/render3/definition.ts

@@ -285,6 +285,7 @@ export function ΔdefineComponent<T>(componentDefinition: {
     _: null as never,
     setInput: null,
     schemas: componentDefinition.schemas || null,
+    tView: null,
   };
   def._ = noSideEffects(() => {
     const directiveTypes = componentDefinition.directives !;

packages/core/src/render3/instructions/shared.ts

@@ -548,29 +548,13 @@ function saveResolvedLocalsInData(
  * Gets TView from a template function or creates a new TView
  * if it doesn't already exist.
  *
- * @param templateFn The template from which to get static data
- * @param consts The number of nodes, local refs, and pipes in this view
- * @param vars The number of bindings and pure function bindings in this view
- * @param directives Directive defs that should be saved on TView
- * @param pipes Pipe defs that should be saved on TView
- * @param viewQuery View query that should be saved on TView
- * @param schemas Schemas that should be saved on TView
+ * @param def ComponentDef
  * @returns TView
  */
-export function getOrCreateTView(
-    templateFn: ComponentTemplate<any>, consts: number, vars: number,
-    directives: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null,
-    viewQuery: ViewQueriesFunction<any>| null, schemas: SchemaMetadata[] | null): TView {
-  // TODO(misko): reading `ngPrivateData` here is problematic for two reasons
-  // 1. It is a megamorphic call on each invocation.
-  // 2. For nested embedded views (ngFor inside ngFor) the template instance is per
-  //    outer template invocation, which means that no such property will exist
-  // Correct solution is to only put `ngPrivateData` on the Component template
-  // and not on embedded templates.
-
-  return templateFn.ngPrivateData ||
-      (templateFn.ngPrivateData = createTView(
-           -1, templateFn, consts, vars, directives, pipes, viewQuery, schemas) as never);
+export function getOrCreateTView(def: ComponentDef<any>): TView {
+  return def.tView || (def.tView = createTView(
+                           -1, def.template, def.consts, def.vars, def.directiveDefs, def.pipeDefs,
+                           def.viewQuery, def.schemas));
 }
 
 /**
@@ -1262,9 +1246,7 @@ function addComponentLogic<T>(
     lView: LView, previousOrParentTNode: TNode, def: ComponentDef<T>): void {
   const native = getNativeByTNode(previousOrParentTNode, lView);
 
-  const tView = getOrCreateTView(
-      def.template, def.consts, def.vars, def.directiveDefs, def.pipeDefs, def.viewQuery,
-      def.schemas);
+  const tView = getOrCreateTView(def);
 
   // Only component views should be added to the view tree directly. Embedded views are
   // accessed through their containers because they may be removed / re-added later.

packages/core/src/render3/interfaces/definition.ts

@@ -10,6 +10,7 @@ import {SchemaMetadata, ViewEncapsulation} from '../../core';
 import {ProcessProvidersFunction} from '../../di/interface/provider';
 import {Type} from '../../interface/type';
 import {CssSelectorList} from './projection';
+import {TView} from './view';
 
 
 /**
@@ -19,7 +20,7 @@ export type ComponentTemplate<T> = {
   // Note: the ctx parameter is typed as T|U, as using only U would prevent a template with
   // e.g. ctx: {} from being assigned to ComponentTemplate<any> as TypeScript won't infer U = any
   // in that scenario. By including T this incompatibility is resolved.
-  <U extends T>(rf: RenderFlags, ctx: T | U): void; ngPrivateData?: never;
+  <U extends T>(rf: RenderFlags, ctx: T | U): void;
 };
 
 /**
@@ -302,6 +303,12 @@ export interface ComponentDef<T> extends DirectiveDef<T> {
    */
   schemas: SchemaMetadata[]|null;
 
+  /**
+   * Ivy runtime uses this place to store the computed tView for the component. This gets filled on
+   * the first run of component.
+   */
+  tView: TView|null;
+
   /**
    * Used to store the result of `noSideEffects` function so that it is not removed by closure
    * compiler. The property should never be read.

packages/core/src/render3/interfaces/view.ts

@@ -327,7 +327,7 @@ export interface ExpandoInstructions extends Array<number|HostBindingsFunction<a
  * The static data for an LView (shared between all templates of a
  * given type).
  *
- * Stored on the template function as ngPrivateData.
+ * Stored on the `ComponentDef.tView`.
  */
 export interface TView {
   /**

packages/core/src/render3/jit/module.ts

@@ -361,7 +361,7 @@ export function patchComponentDefWithScope<C>(
   // may face a problem where previously compiled defs available to a given Component/Directive
   // are cached in TView and may become stale (in case any of these defs gets recompiled). In
   // order to avoid this problem, we force fresh TView to be created.
-  componentDef.template.ngPrivateData = undefined;
+  componentDef.tView = null;
 }
 
 /**

packages/core/test/acceptance/integration_spec.ts

@@ -7,6 +7,7 @@
  */
 import {CommonModule} from '@angular/common';
 import {Component, ContentChild, Directive, ElementRef, EventEmitter, HostBinding, HostListener, Input, OnInit, Output, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
+import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
 import {TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
@@ -29,23 +30,39 @@ describe('acceptance integration tests', () => {
     });
 
     it('should render and update basic "Hello, World" template', () => {
+      ngDevModeResetPerfCounters();
       @Component({template: '<h1>Hello, {{name}}!</h1>'})
       class App {
         name = '';
       }
 
+      onlyInIvy('perf counters').expectPerfCounters({
+        tView: 0,
+        tNode: 0,
+      });
+
       TestBed.configureTestingModule({declarations: [App]});
       const fixture = TestBed.createComponent(App);
 
       fixture.componentInstance.name = 'World';
       fixture.detectChanges();
 
       expect(fixture.nativeElement.innerHTML).toEqual('<h1>Hello, World!</h1>');
+      onlyInIvy('perf counters').expectPerfCounters({
+        tView: 2,  // Host view + App
+        tNode: 4,  // Host Node + App Node + <span> + #text
+      });
 
       fixture.componentInstance.name = 'New World';
       fixture.detectChanges();
 
       expect(fixture.nativeElement.innerHTML).toEqual('<h1>Hello, New World!</h1>');
+      // Assert that the tView/tNode count does not increase (they are correctly cached)
+      onlyInIvy('perf counters').expectPerfCounters({
+        tView: 2,
+        tNode: 4,
+      });
+
     });
   });
 

packages/core/test/linker/ng_module_integration_spec.ts

@@ -142,7 +142,7 @@ function declareTests(config?: {useJit: boolean}) {
         // may face a problem where previously compiled defs available to a given
         // Component/Directive are cached in TView and may become stale (in case any of these defs
         // gets recompiled). In order to avoid this problem, we force fresh TView to be created.
-        componentDef.template.ngPrivateData = null;
+        componentDef.TView = null;
       }
 
       const ngModule = createModule(moduleType, injector);

packages/core/test/render3/integration_spec.ts

@@ -196,56 +196,6 @@ describe('render3 integration test', () => {
 
 });
 
-describe('template data', () => {
-
-  it('should re-use template data and node data', () => {
-    /**
-     *  % if (condition) {
-     *    <div></div>
-     *  % }
-     */
-    function Template(rf: RenderFlags, ctx: any) {
-      if (rf & RenderFlags.Create) {
-        Δcontainer(0);
-      }
-      if (rf & RenderFlags.Update) {
-        ΔcontainerRefreshStart(0);
-        {
-          if (ctx.condition) {
-            let rf1 = ΔembeddedViewStart(0, 1, 0);
-            if (rf1 & RenderFlags.Create) {
-              Δelement(0, 'div');
-            }
-            ΔembeddedViewEnd();
-          }
-        }
-        ΔcontainerRefreshEnd();
-      }
-    }
-
-    expect((Template as any).ngPrivateData).toBeUndefined();
-
-    renderToHtml(Template, {condition: true}, 1);
-
-    const oldTemplateData = (Template as any).ngPrivateData;
-    const oldContainerData = (oldTemplateData as any).data[HEADER_OFFSET];
-    const oldElementData = oldContainerData.tViews[0][HEADER_OFFSET];
-    expect(oldContainerData).not.toBeNull();
-    expect(oldElementData).not.toBeNull();
-
-    renderToHtml(Template, {condition: false}, 1);
-    renderToHtml(Template, {condition: true}, 1);
-
-    const newTemplateData = (Template as any).ngPrivateData;
-    const newContainerData = (oldTemplateData as any).data[HEADER_OFFSET];
-    const newElementData = oldContainerData.tViews[0][HEADER_OFFSET];
-    expect(newTemplateData === oldTemplateData).toBe(true);
-    expect(newContainerData === oldContainerData).toBe(true);
-    expect(newElementData === oldElementData).toBe(true);
-  });
-
-});
-
 describe('component styles', () => {
   it('should pass in the component styles directly into the underlying renderer', () => {
     class StyledComp {

packages/core/test/render3/render_util.ts

@@ -28,7 +28,7 @@ import {CreateComponentOptions} from '../../src/render3/component';
 import {getDirectivesAtNodeIndex, getLContext, isComponentInstance} from '../../src/render3/context_discovery';
 import {extractDirectiveDef, extractPipeDef} from '../../src/render3/definition';
 import {NG_ELEMENT_ID} from '../../src/render3/fields';
-import {ComponentTemplate, ComponentType, DirectiveDef, DirectiveType, RenderFlags, renderComponent as _renderComponent, tick, ΔProvidersFeature, ΔdefineComponent, ΔdefineDirective} from '../../src/render3/index';
+import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveType, RenderFlags, renderComponent as _renderComponent, tick, ΔProvidersFeature, ΔdefineComponent, ΔdefineDirective} from '../../src/render3/index';
 import {DirectiveDefList, DirectiveDefListOrFactory, DirectiveTypesOrFactory, HostBindingsFunction, PipeDef, PipeDefList, PipeDefListOrFactory, PipeTypesOrFactory} from '../../src/render3/interfaces/definition';
 import {PlayerHandler} from '../../src/render3/interfaces/player';
 import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, RendererStyleFlags3, domRendererFactory3} from '../../src/render3/interfaces/renderer';
@@ -258,8 +258,18 @@ export function renderTemplate<T>(
         LViewFlags.CheckAlways | LViewFlags.IsRoot, null, null, providedRendererFactory, renderer);
     enterView(hostLView, null);  // SUSPECT! why do we need to enter the View?
 
-    const componentTView =
-        getOrCreateTView(templateFn, consts, vars, directives || null, pipes || null, null, null);
+    const def: ComponentDef<any> = ΔdefineComponent({
+      factory: () => null,
+      selectors: [],
+      type: Object,
+      template: templateFn,
+      consts: consts,
+      vars: vars,
+    });
+    def.directiveDefs = directives || null;
+    def.pipeDefs = pipes || null;
+
+    const componentTView = getOrCreateTView(def);
     const hostTNode = createNodeAtIndex(0, TNodeType.Element, hostNode, null, null);
     componentView = createLView(
         hostLView, componentTView, context, LViewFlags.CheckAlways, hostNode, hostTNode,