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

PR Close #30548
This commit is contained in:
Miško Hevery 2019-05-17 16:25:09 -07:00 committed by Jason Aden
parent 28ae22ecb9
commit 6454f76cf6
10 changed files with 50 additions and 86 deletions

View File

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

View File

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

View File

@ -548,29 +548,13 @@ function saveResolvedLocalsInData(
* Gets TView from a template function or creates a new TView * Gets TView from a template function or creates a new TView
* if it doesn't already exist. * if it doesn't already exist.
* *
* @param templateFn The template from which to get static data * @param def ComponentDef
* @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
* @returns TView * @returns TView
*/ */
export function getOrCreateTView( export function getOrCreateTView(def: ComponentDef<any>): TView {
templateFn: ComponentTemplate<any>, consts: number, vars: number, return def.tView || (def.tView = createTView(
directives: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null, -1, def.template, def.consts, def.vars, def.directiveDefs, def.pipeDefs,
viewQuery: ViewQueriesFunction<any>| null, schemas: SchemaMetadata[] | null): TView { def.viewQuery, def.schemas));
// 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);
} }
/** /**
@ -1262,9 +1246,7 @@ function addComponentLogic<T>(
lView: LView, previousOrParentTNode: TNode, def: ComponentDef<T>): void { lView: LView, previousOrParentTNode: TNode, def: ComponentDef<T>): void {
const native = getNativeByTNode(previousOrParentTNode, lView); const native = getNativeByTNode(previousOrParentTNode, lView);
const tView = getOrCreateTView( const tView = getOrCreateTView(def);
def.template, def.consts, def.vars, def.directiveDefs, def.pipeDefs, def.viewQuery,
def.schemas);
// Only component views should be added to the view tree directly. Embedded views are // 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. // accessed through their containers because they may be removed / re-added later.

View File

@ -10,6 +10,7 @@ import {SchemaMetadata, ViewEncapsulation} from '../../core';
import {ProcessProvidersFunction} from '../../di/interface/provider'; import {ProcessProvidersFunction} from '../../di/interface/provider';
import {Type} from '../../interface/type'; import {Type} from '../../interface/type';
import {CssSelectorList} from './projection'; 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 // 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 // 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. // 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; 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 * Used to store the result of `noSideEffects` function so that it is not removed by closure
* compiler. The property should never be read. * compiler. The property should never be read.

View File

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

View File

@ -361,7 +361,7 @@ export function patchComponentDefWithScope<C>(
// may face a problem where previously compiled defs available to a given Component/Directive // 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 // 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. // order to avoid this problem, we force fresh TView to be created.
componentDef.template.ngPrivateData = undefined; componentDef.tView = null;
} }
/** /**

View File

@ -7,6 +7,7 @@
*/ */
import {CommonModule} from '@angular/common'; 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 {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 {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser'; import {By} from '@angular/platform-browser';
import {expect} from '@angular/platform-browser/testing/src/matchers'; import {expect} from '@angular/platform-browser/testing/src/matchers';
@ -29,11 +30,17 @@ describe('acceptance integration tests', () => {
}); });
it('should render and update basic "Hello, World" template', () => { it('should render and update basic "Hello, World" template', () => {
ngDevModeResetPerfCounters();
@Component({template: '<h1>Hello, {{name}}!</h1>'}) @Component({template: '<h1>Hello, {{name}}!</h1>'})
class App { class App {
name = ''; name = '';
} }
onlyInIvy('perf counters').expectPerfCounters({
tView: 0,
tNode: 0,
});
TestBed.configureTestingModule({declarations: [App]}); TestBed.configureTestingModule({declarations: [App]});
const fixture = TestBed.createComponent(App); const fixture = TestBed.createComponent(App);
@ -41,11 +48,21 @@ describe('acceptance integration tests', () => {
fixture.detectChanges(); fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toEqual('<h1>Hello, World!</h1>'); 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.componentInstance.name = 'New World';
fixture.detectChanges(); fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toEqual('<h1>Hello, New World!</h1>'); 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,
});
}); });
}); });

View File

@ -142,7 +142,7 @@ function declareTests(config?: {useJit: boolean}) {
// may face a problem where previously compiled defs available to a given // 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 // 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. // 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); const ngModule = createModule(moduleType, injector);

View File

@ -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', () => { describe('component styles', () => {
it('should pass in the component styles directly into the underlying renderer', () => { it('should pass in the component styles directly into the underlying renderer', () => {
class StyledComp { class StyledComp {

View File

@ -28,7 +28,7 @@ import {CreateComponentOptions} from '../../src/render3/component';
import {getDirectivesAtNodeIndex, getLContext, isComponentInstance} from '../../src/render3/context_discovery'; import {getDirectivesAtNodeIndex, getLContext, isComponentInstance} from '../../src/render3/context_discovery';
import {extractDirectiveDef, extractPipeDef} from '../../src/render3/definition'; import {extractDirectiveDef, extractPipeDef} from '../../src/render3/definition';
import {NG_ELEMENT_ID} from '../../src/render3/fields'; 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 {DirectiveDefList, DirectiveDefListOrFactory, DirectiveTypesOrFactory, HostBindingsFunction, PipeDef, PipeDefList, PipeDefListOrFactory, PipeTypesOrFactory} from '../../src/render3/interfaces/definition';
import {PlayerHandler} from '../../src/render3/interfaces/player'; import {PlayerHandler} from '../../src/render3/interfaces/player';
import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, RendererStyleFlags3, domRendererFactory3} from '../../src/render3/interfaces/renderer'; 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); LViewFlags.CheckAlways | LViewFlags.IsRoot, null, null, providedRendererFactory, renderer);
enterView(hostLView, null); // SUSPECT! why do we need to enter the View? enterView(hostLView, null); // SUSPECT! why do we need to enter the View?
const componentTView = const def: ComponentDef<any> = ΔdefineComponent({
getOrCreateTView(templateFn, consts, vars, directives || null, pipes || null, null, null); 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); const hostTNode = createNodeAtIndex(0, TNodeType.Element, hostNode, null, null);
componentView = createLView( componentView = createLView(
hostLView, componentTView, context, LViewFlags.CheckAlways, hostNode, hostTNode, hostLView, componentTView, context, LViewFlags.CheckAlways, hostNode, hostTNode,