Skip to content

Commit 8154433

Browse files
pkozlowski-opensourcemhevery
authored andcommitted
perf(ivy): limit allocation of LQueries_ objects (angular#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 angular#30664
1 parent 07cd65b commit 8154433

File tree

4 files changed

+50
-20
lines changed

4 files changed

+50
-20
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export function ɵɵelementStart(
125125
const currentQueries = lView[QUERIES];
126126
if (currentQueries) {
127127
currentQueries.addNode(tNode);
128-
lView[QUERIES] = currentQueries.clone();
128+
lView[QUERIES] = currentQueries.clone(tNode);
129129
}
130130
executeContentQueries(tView, tNode, lView);
131131
}
@@ -153,7 +153,8 @@ export function ɵɵelementEnd(): void {
153153
ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.Element);
154154
const lView = getLView();
155155
const currentQueries = lView[QUERIES];
156-
if (currentQueries) {
156+
// Go back up to parent queries only if queries have been cloned on this element.
157+
if (currentQueries && previousOrParentTNode.index === currentQueries.nodeIndex) {
157158
lView[QUERIES] = currentQueries.parent;
158159
}
159160

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export function ɵɵelementContainerStart(
6565
const currentQueries = lView[QUERIES];
6666
if (currentQueries) {
6767
currentQueries.addNode(tNode);
68-
lView[QUERIES] = currentQueries.clone();
68+
lView[QUERIES] = currentQueries.clone(tNode);
6969
}
7070
executeContentQueries(tView, tNode, lView);
7171
}
@@ -89,7 +89,8 @@ export function ɵɵelementContainerEnd(): void {
8989

9090
ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.ElementContainer);
9191
const currentQueries = lView[QUERIES];
92-
if (currentQueries) {
92+
// Go back up to parent queries only if queries have been cloned on this element.
93+
if (currentQueries && previousOrParentTNode.index === currentQueries.nodeIndex) {
9394
lView[QUERIES] = currentQueries.parent;
9495
}
9596

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,32 @@ export interface LQueries {
2525
parent: LQueries|null;
2626

2727
/**
28-
* Ask queries to prepare copy of itself. This assures that tracking new queries on content nodes
29-
* doesn't mutate list of queries tracked on a parent node. We will clone LQueries before
30-
* constructing content queries.
28+
* The index of the node on which this LQueries instance was created / cloned in a given LView.
29+
*
30+
* This index is stored to minimize LQueries cloning: we can observe that LQueries can be mutated
31+
* only under 2 conditions:
32+
* - we are crossing an element that has directives with content queries (new queries are added);
33+
* - we are descending into element hierarchy (creating a child element of an existing element)
34+
* and the current LQueries object is tracking shallow queries (shallow queries are removed).
35+
*
36+
* Since LQueries are not cloned systematically we need to know exactly where (on each element)
37+
* cloning occurred, so we can properly restore the set of tracked queries when going up the
38+
* elements hierarchy.
39+
*
40+
* Always set to -1 for view queries as view queries are created before we process any node in a
41+
* given view.
42+
*/
43+
nodeIndex: number;
44+
45+
/**
46+
* Ask queries to prepare a copy of itself. This ensures that:
47+
* - tracking new queries on content nodes doesn't mutate list of queries tracked on a parent
48+
* node;
49+
* - we don't track shallow queries when descending into elements hierarchy.
50+
*
51+
* We will clone LQueries before constructing content queries
3152
*/
32-
clone(): LQueries;
53+
clone(tNode: TNode): LQueries;
3354

3455
/**
3556
* Notify `LQueries` that a new `TNode` has been created and needs to be added to query results

packages/core/src/render3/query.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import {unusedValueExportToPlacateAjd as unused2} from './interfaces/injector';
2525
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node';
2626
import {LQueries, unusedValueExportToPlacateAjd as unused4} from './interfaces/query';
2727
import {CONTENT_QUERIES, HEADER_OFFSET, LView, QUERIES, TVIEW, TView} from './interfaces/view';
28-
import {getCurrentQueryIndex, getIsParent, getLView, isCreationMode, setCurrentQueryIndex} from './state';
29-
import {loadInternal} from './util/view_utils';
28+
import {getCurrentQueryIndex, getIsParent, getLView, getPreviousOrParentTNode, isCreationMode, setCurrentQueryIndex} from './state';
29+
import {isContentQueryHost, loadInternal} from './util/view_utils';
3030
import {createElementRef, createTemplateRef} from './view_engine_compatibility';
3131

3232
const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4;
@@ -92,7 +92,7 @@ class LQuery<T> {
9292
export class LQueries_ implements LQueries {
9393
constructor(
9494
public parent: LQueries_|null, private shallow: LQuery<any>|null,
95-
private deep: LQuery<any>|null) {}
95+
private deep: LQuery<any>|null, public nodeIndex: number = -1) {}
9696

9797
track<T>(queryList: QueryList<T>, predicate: Type<T>|string[], descend?: boolean, read?: Type<T>):
9898
void {
@@ -103,7 +103,11 @@ export class LQueries_ implements LQueries {
103103
}
104104
}
105105

106-
clone(): LQueries { return new LQueries_(this, null, this.deep); }
106+
clone(tNode: TNode): LQueries {
107+
return this.shallow !== null || isContentQueryHost(tNode) ?
108+
new LQueries_(this, null, this.deep, tNode.index) :
109+
this;
110+
}
107111

108112
container(): LQueries|null {
109113
const shallowResults = copyQueriesToContainer(this.shallow);
@@ -352,11 +356,11 @@ type QueryList_<T> = QueryList<T>& {_valuesTree: any[], _static: boolean};
352356
*/
353357
function createQueryListInLView<T>(
354358
// TODO: "read" should be an AbstractType (FW-486)
355-
lView: LView, predicate: Type<any>| string[], descend: boolean, read: any,
356-
isStatic: boolean): QueryList<T> {
359+
lView: LView, predicate: Type<any>| string[], descend: boolean, read: any, isStatic: boolean,
360+
nodeIndex: number): QueryList<T> {
357361
ngDevMode && assertPreviousIsParent(getIsParent());
358362
const queryList = new QueryList<T>() as QueryList_<T>;
359-
const queries = lView[QUERIES] || (lView[QUERIES] = new LQueries_(null, null, null));
363+
const queries = lView[QUERIES] || (lView[QUERIES] = new LQueries_(null, null, null, nodeIndex));
360364
queryList._valuesTree = [];
361365
queryList._static = isStatic;
362366
queries.track(queryList, predicate, descend, read);
@@ -430,7 +434,7 @@ function viewQueryInternal<T>(
430434
}
431435
const index = getCurrentQueryIndex();
432436
const queryList: QueryList<T> =
433-
createQueryListInLView<T>(lView, predicate, descend, read, isStatic);
437+
createQueryListInLView<T>(lView, predicate, descend, read, isStatic, -1);
434438
store(index - HEADER_OFFSET, queryList);
435439
setCurrentQueryIndex(index + 1);
436440
return queryList;
@@ -465,16 +469,18 @@ export function ɵɵcontentQuery<T>(
465469
read: any): QueryList<T> {
466470
const lView = getLView();
467471
const tView = lView[TVIEW];
468-
return contentQueryInternal(lView, tView, directiveIndex, predicate, descend, read, false);
472+
const tNode = getPreviousOrParentTNode();
473+
return contentQueryInternal(
474+
lView, tView, directiveIndex, predicate, descend, read, false, tNode.index);
469475
}
470476

471477
function contentQueryInternal<T>(
472478
lView: LView, tView: TView, directiveIndex: number, predicate: Type<any>| string[],
473479
descend: boolean,
474480
// TODO(FW-486): "read" should be an AbstractType
475-
read: any, isStatic: boolean): QueryList<T> {
481+
read: any, isStatic: boolean, nodeIndex: number): QueryList<T> {
476482
const contentQuery: QueryList<T> =
477-
createQueryListInLView<T>(lView, predicate, descend, read, isStatic);
483+
createQueryListInLView<T>(lView, predicate, descend, read, isStatic, nodeIndex);
478484
(lView[CONTENT_QUERIES] || (lView[CONTENT_QUERIES] = [])).push(contentQuery);
479485
if (tView.firstTemplatePass) {
480486
const tViewContentQueries = tView.contentQueries || (tView.contentQueries = []);
@@ -505,7 +511,8 @@ export function ɵɵstaticContentQuery<T>(
505511
read: any): void {
506512
const lView = getLView();
507513
const tView = lView[TVIEW];
508-
contentQueryInternal(lView, tView, directiveIndex, predicate, descend, read, true);
514+
const tNode = getPreviousOrParentTNode();
515+
contentQueryInternal(lView, tView, directiveIndex, predicate, descend, read, true, tNode.index);
509516
tView.staticContentQueries = true;
510517
}
511518

0 commit comments

Comments
 (0)