From 815443313044e0ec0ec87af588d09eda6b88f0c8 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 24 May 2019 17:47:21 +0200 Subject: [PATCH] perf(ivy): limit allocation of LQueries_ objects (#30664) Before this change we would systematically call LQueries.clone() when executting elementStart / elementContainerStart instructions. This was often unnecessary as LQueries can be mutated under 2 conditions only: - we are crossing an element that has directives with content queries (new queries must be added); - we are descending into element hierarchy (creating a child element of an existing element) and the current LQueries object is tracking shallow queries (shallow queries are removed). With this PR LQueires.clone() is only done when needed and this gratelly reduces number of LQueries object created: in the "expanding rows" benchmark number of allocated (and often GCed just after!) LQueries is reduced from ~100k -> ~35k. This represents over 1MB of memory that is not allocated. PR Close #30664 --- .../core/src/render3/instructions/element.ts | 5 +-- .../render3/instructions/element_container.ts | 5 +-- packages/core/src/render3/interfaces/query.ts | 29 ++++++++++++++--- packages/core/src/render3/query.ts | 31 ++++++++++++------- 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index f95a582d87..1cdc428251 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -125,7 +125,7 @@ export function ɵɵelementStart( const currentQueries = lView[QUERIES]; if (currentQueries) { currentQueries.addNode(tNode); - lView[QUERIES] = currentQueries.clone(); + lView[QUERIES] = currentQueries.clone(tNode); } executeContentQueries(tView, tNode, lView); } @@ -153,7 +153,8 @@ export function ɵɵelementEnd(): void { ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.Element); const lView = getLView(); const currentQueries = lView[QUERIES]; - if (currentQueries) { + // Go back up to parent queries only if queries have been cloned on this element. + if (currentQueries && previousOrParentTNode.index === currentQueries.nodeIndex) { lView[QUERIES] = currentQueries.parent; } diff --git a/packages/core/src/render3/instructions/element_container.ts b/packages/core/src/render3/instructions/element_container.ts index 1c38196a91..456051db21 100644 --- a/packages/core/src/render3/instructions/element_container.ts +++ b/packages/core/src/render3/instructions/element_container.ts @@ -65,7 +65,7 @@ export function ɵɵelementContainerStart( const currentQueries = lView[QUERIES]; if (currentQueries) { currentQueries.addNode(tNode); - lView[QUERIES] = currentQueries.clone(); + lView[QUERIES] = currentQueries.clone(tNode); } executeContentQueries(tView, tNode, lView); } @@ -89,7 +89,8 @@ export function ɵɵelementContainerEnd(): void { ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.ElementContainer); const currentQueries = lView[QUERIES]; - if (currentQueries) { + // Go back up to parent queries only if queries have been cloned on this element. + if (currentQueries && previousOrParentTNode.index === currentQueries.nodeIndex) { lView[QUERIES] = currentQueries.parent; } diff --git a/packages/core/src/render3/interfaces/query.ts b/packages/core/src/render3/interfaces/query.ts index 9aad007f98..0615ec7946 100644 --- a/packages/core/src/render3/interfaces/query.ts +++ b/packages/core/src/render3/interfaces/query.ts @@ -25,11 +25,32 @@ export interface LQueries { parent: LQueries|null; /** - * Ask queries to prepare copy of itself. This assures that tracking new queries on content nodes - * doesn't mutate list of queries tracked on a parent node. We will clone LQueries before - * constructing content queries. + * The index of the node on which this LQueries instance was created / cloned in a given LView. + * + * This index is stored to minimize LQueries cloning: we can observe that LQueries can be mutated + * only under 2 conditions: + * - we are crossing an element that has directives with content queries (new queries are added); + * - we are descending into element hierarchy (creating a child element of an existing element) + * and the current LQueries object is tracking shallow queries (shallow queries are removed). + * + * Since LQueries are not cloned systematically we need to know exactly where (on each element) + * cloning occurred, so we can properly restore the set of tracked queries when going up the + * elements hierarchy. + * + * Always set to -1 for view queries as view queries are created before we process any node in a + * given view. */ - clone(): LQueries; + nodeIndex: number; + + /** + * Ask queries to prepare a copy of itself. This ensures that: + * - tracking new queries on content nodes doesn't mutate list of queries tracked on a parent + * node; + * - we don't track shallow queries when descending into elements hierarchy. + * + * We will clone LQueries before constructing content queries + */ + clone(tNode: TNode): LQueries; /** * Notify `LQueries` that a new `TNode` has been created and needs to be added to query results diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 18a5bd4183..81bb75bab1 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -25,8 +25,8 @@ import {unusedValueExportToPlacateAjd as unused2} from './interfaces/injector'; import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node'; import {LQueries, unusedValueExportToPlacateAjd as unused4} from './interfaces/query'; import {CONTENT_QUERIES, HEADER_OFFSET, LView, QUERIES, TVIEW, TView} from './interfaces/view'; -import {getCurrentQueryIndex, getIsParent, getLView, isCreationMode, setCurrentQueryIndex} from './state'; -import {loadInternal} from './util/view_utils'; +import {getCurrentQueryIndex, getIsParent, getLView, getPreviousOrParentTNode, isCreationMode, setCurrentQueryIndex} from './state'; +import {isContentQueryHost, loadInternal} from './util/view_utils'; import {createElementRef, createTemplateRef} from './view_engine_compatibility'; const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4; @@ -92,7 +92,7 @@ class LQuery { export class LQueries_ implements LQueries { constructor( public parent: LQueries_|null, private shallow: LQuery|null, - private deep: LQuery|null) {} + private deep: LQuery|null, public nodeIndex: number = -1) {} track(queryList: QueryList, predicate: Type|string[], descend?: boolean, read?: Type): void { @@ -103,7 +103,11 @@ export class LQueries_ implements LQueries { } } - clone(): LQueries { return new LQueries_(this, null, this.deep); } + clone(tNode: TNode): LQueries { + return this.shallow !== null || isContentQueryHost(tNode) ? + new LQueries_(this, null, this.deep, tNode.index) : + this; + } container(): LQueries|null { const shallowResults = copyQueriesToContainer(this.shallow); @@ -352,11 +356,11 @@ type QueryList_ = QueryList& {_valuesTree: any[], _static: boolean}; */ function createQueryListInLView( // TODO: "read" should be an AbstractType (FW-486) - lView: LView, predicate: Type| string[], descend: boolean, read: any, - isStatic: boolean): QueryList { + lView: LView, predicate: Type| string[], descend: boolean, read: any, isStatic: boolean, + nodeIndex: number): QueryList { ngDevMode && assertPreviousIsParent(getIsParent()); const queryList = new QueryList() as QueryList_; - const queries = lView[QUERIES] || (lView[QUERIES] = new LQueries_(null, null, null)); + const queries = lView[QUERIES] || (lView[QUERIES] = new LQueries_(null, null, null, nodeIndex)); queryList._valuesTree = []; queryList._static = isStatic; queries.track(queryList, predicate, descend, read); @@ -430,7 +434,7 @@ function viewQueryInternal( } const index = getCurrentQueryIndex(); const queryList: QueryList = - createQueryListInLView(lView, predicate, descend, read, isStatic); + createQueryListInLView(lView, predicate, descend, read, isStatic, -1); store(index - HEADER_OFFSET, queryList); setCurrentQueryIndex(index + 1); return queryList; @@ -465,16 +469,18 @@ export function ɵɵcontentQuery( read: any): QueryList { const lView = getLView(); const tView = lView[TVIEW]; - return contentQueryInternal(lView, tView, directiveIndex, predicate, descend, read, false); + const tNode = getPreviousOrParentTNode(); + return contentQueryInternal( + lView, tView, directiveIndex, predicate, descend, read, false, tNode.index); } function contentQueryInternal( lView: LView, tView: TView, directiveIndex: number, predicate: Type| string[], descend: boolean, // TODO(FW-486): "read" should be an AbstractType - read: any, isStatic: boolean): QueryList { + read: any, isStatic: boolean, nodeIndex: number): QueryList { const contentQuery: QueryList = - createQueryListInLView(lView, predicate, descend, read, isStatic); + createQueryListInLView(lView, predicate, descend, read, isStatic, nodeIndex); (lView[CONTENT_QUERIES] || (lView[CONTENT_QUERIES] = [])).push(contentQuery); if (tView.firstTemplatePass) { const tViewContentQueries = tView.contentQueries || (tView.contentQueries = []); @@ -505,7 +511,8 @@ export function ɵɵstaticContentQuery( read: any): void { const lView = getLView(); const tView = lView[TVIEW]; - contentQueryInternal(lView, tView, directiveIndex, predicate, descend, read, true); + const tNode = getPreviousOrParentTNode(); + contentQueryInternal(lView, tView, directiveIndex, predicate, descend, read, true, tNode.index); tView.staticContentQueries = true; }