Skip to content

Commit

Permalink
perf(ivy): avoid repeated native node retrieval and patching (#33322)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pkozlowski-opensource authored and atscott committed Oct 31, 2019
1 parent 2c208f9 commit 41caafc
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 36 deletions.
45 changes: 18 additions & 27 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -1018,17 +1018,20 @@ function warnAboutUnknownProperty(propName: string, tNode: TNode): void {
/**
* Instantiate a root component.
*/
export function instantiateRootComponent<T>(
tView: TView, viewData: LView, def: ComponentDef<T>): T {
export function instantiateRootComponent<T>(tView: TView, lView: LView, def: ComponentDef<T>): 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;
}

Expand Down Expand Up @@ -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<any>;
const isComponent = isComponentDef(def);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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<T>(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.
Expand Down
Expand Up @@ -392,9 +392,6 @@
{
"name": "noSideEffects"
},
{
"name": "postProcessBaseDirective"
},
{
"name": "refreshChildComponents"
},
Expand Down
6 changes: 0 additions & 6 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -1088,12 +1088,6 @@
{
"name": "patchConfig"
},
{
"name": "postProcessBaseDirective"
},
{
"name": "postProcessDirective"
},
{
"name": "readPatchedData"
},
Expand Down

0 comments on commit 41caafc

Please sign in to comment.