From 41caafcaf2bcd088454624cba7ef1671c1933fee Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Mon, 21 Oct 2019 10:38:21 +0200 Subject: [PATCH] perf(ivy): avoid repeated native node retrieval and patching (#33322) Before this change instantiating multiple directives on the same host node would result in repeated RNode retrieval and patching. This commint re-organises code around directive instance creation so the host node processing (common to all directives) happens once and only once. As the additional benefit the directive instantiation logic gets centralised in one function (at the expense of patching logic duplication for root node). PR Close #33322 --- .../core/src/render3/instructions/shared.ts | 45 ++++++++----------- .../hello_world/bundle.golden_symbols.json | 3 -- .../bundling/todo/bundle.golden_symbols.json | 6 --- 3 files changed, 18 insertions(+), 36 deletions(-) diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index d25c001790..d291bb7a4c 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -24,7 +24,7 @@ import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/c import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition'; import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from '../interfaces/injector'; import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node'; -import {RComment, RElement, RText, Renderer3, RendererFactory3, isProceduralRenderer} from '../interfaces/renderer'; +import {RComment, RElement, RNode, RText, Renderer3, RendererFactory3, isProceduralRenderer} from '../interfaces/renderer'; import {SanitizerFn} from '../interfaces/sanitization'; import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks'; import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TData, TVIEW, TView, T_HOST} from '../interfaces/view'; @@ -529,10 +529,9 @@ export function executeContentQueries(tView: TView, tNode: TNode, lView: LView) /** * Creates directive instances. */ -export function createDirectivesInstances( - tView: TView, lView: LView, tNode: TElementNode | TContainerNode | TElementContainerNode) { +export function createDirectivesInstances(tView: TView, lView: LView, tNode: TDirectiveHostNode) { if (!getBindingsEnabled()) return; - instantiateAllDirectives(tView, lView, tNode); + instantiateAllDirectives(tView, lView, tNode, getNativeByTNode(tNode, lView)); if ((tNode.flags & TNodeFlags.hasHostBindings) === TNodeFlags.hasHostBindings) { invokeDirectivesHostBindings(tView, lView, tNode); } @@ -543,7 +542,8 @@ export function createDirectivesInstances( * to LView in the same order as they are loaded in the template with load(). */ export function saveResolvedLocalsInData( - viewData: LView, tNode: TNode, localRefExtractor: LocalRefExtractor = getNativeByTNode): void { + viewData: LView, tNode: TDirectiveHostNode, + localRefExtractor: LocalRefExtractor = getNativeByTNode): void { const localNames = tNode.localNames; if (localNames) { let localIndex = tNode.index + 1; @@ -1018,17 +1018,20 @@ function warnAboutUnknownProperty(propName: string, tNode: TNode): void { /** * Instantiate a root component. */ -export function instantiateRootComponent( - tView: TView, viewData: LView, def: ComponentDef): T { +export function instantiateRootComponent(tView: TView, lView: LView, def: ComponentDef): T { const rootTNode = getPreviousOrParentTNode(); if (tView.firstTemplatePass) { if (def.providersResolver) def.providersResolver(def); generateExpandoInstructionBlock(tView, rootTNode, 1); - baseResolveDirective(tView, viewData, def); + baseResolveDirective(tView, lView, def); } const directive = - getNodeInjectable(tView.data, viewData, viewData.length - 1, rootTNode as TElementNode); - postProcessBaseDirective(viewData, rootTNode, directive); + getNodeInjectable(tView.data, lView, lView.length - 1, rootTNode as TElementNode); + attachPatchData(directive, lView); + const native = getNativeByTNode(rootTNode, lView); + if (native) { + attachPatchData(native, lView); + } return directive; } @@ -1090,13 +1093,16 @@ export function resolveDirectives( /** * Instantiate all the directives that were previously resolved on the current node. */ -function instantiateAllDirectives(tView: TView, lView: LView, tNode: TDirectiveHostNode) { +function instantiateAllDirectives( + tView: TView, lView: LView, tNode: TDirectiveHostNode, native: RNode) { const start = tNode.directiveStart; const end = tNode.directiveEnd; if (!tView.firstTemplatePass) { getOrCreateNodeInjectorForNode(tNode, lView); } + attachPatchData(native, lView); + for (let i = start; i < end; i++) { const def = tView.data[i] as DirectiveDef; const isComponent = isComponentDef(def); @@ -1107,8 +1113,8 @@ function instantiateAllDirectives(tView: TView, lView: LView, tNode: TDirectiveH } const directive = getNodeInjectable(tView.data, lView, i, tNode); + attachPatchData(directive, lView); - postProcessBaseDirective(lView, tNode, directive); if (tNode.initialInputs !== null) { setInputsFromAttrs(lView, i - start, directive, def, tNode); } @@ -1182,21 +1188,6 @@ export function generateExpandoInstructionBlock( ])).push(elementIndex, providerCount, directiveCount); } -/** - * A lighter version of postProcessDirective() that is used for the root component. - */ -function postProcessBaseDirective(lView: LView, hostTNode: TNode, directive: T): void { - ngDevMode && assertLessThanOrEqual( - getBindingIndex(), lView[TVIEW].bindingStartIndex, - 'directives should be created before any bindings'); - attachPatchData(directive, lView); - const native = getNativeByTNode(hostTNode, lView); - if (native) { - attachPatchData(native, lView); - } -} - - /** * Matches the current node against all available selectors. * If a component is matched (at most one), it is returned in first position in the array. diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 2b3924e09d..46ff0fb4bf 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -392,9 +392,6 @@ { "name": "noSideEffects" }, - { - "name": "postProcessBaseDirective" - }, { "name": "refreshChildComponents" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index cebecb6d3b..90a2e991ee 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1088,12 +1088,6 @@ { "name": "patchConfig" }, - { - "name": "postProcessBaseDirective" - }, - { - "name": "postProcessDirective" - }, { "name": "readPatchedData" },