Skip to content

Commit

Permalink
fix(core): avoid storing LView in __ngContext__
Browse files Browse the repository at this point in the history
Currently we save a reference to an `LView` on most DOM nodes created by Angular either by saving
the `LView` directly in the `__ngContext__` or by saving the `LContext` which has a reference to
the `LView`. This can be a problem if the DOM node is retained in memory, because the `LView` has
references to all of the child nodes of the view, as well as other internal data structures.

Previously we tried to resolve the issue by clearing the `__ngContext__` when a node is removed
(see #36011), but we decided not to proceeed, because it can
slow down destruction due to a megamorphic write.

These changes aim to address the issue while reducing the performance impact by assigning a unique
ID when an `LView` is created and adding it to `__ngContext__`. All active views are tracked in
an array where their unique ID corresponds to an index. Using an array should guarantee fast
creation, destruction and lookups. We don't need to worry about leaks within that array, because
`LView`s are an internal data structure and we have complete control over when they are created
and destroyed.

Fixes #41047.
  • Loading branch information
crisbeto committed Mar 29, 2021
1 parent b61c009 commit 7038e66
Show file tree
Hide file tree
Showing 33 changed files with 493 additions and 342 deletions.
26 changes: 16 additions & 10 deletions packages/core/src/debug/debug_node.ts
Expand Up @@ -7,7 +7,8 @@
*/

import {Injector} from '../di/injector';
import {assertTNodeForLView} from '../render3/assert';
import {assertLView, assertTNodeForLView} from '../render3/assert';
import {getLViewById} from '../render3/instructions/lview_tracking';
import {CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from '../render3/interfaces/container';
import {TElementNode, TNode, TNodeFlags, TNodeType} from '../render3/interfaces/node';
import {isComponentHost, isLContainer} from '../render3/interfaces/type_checks';
Expand Down Expand Up @@ -263,8 +264,9 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
get name(): string {
try {
const context = loadLContext(this.nativeNode)!;
const lView = context.lView;
const tData = lView[TVIEW].data;
const lView = getLViewById(context.lViewId);
ngDevMode && assertLView(lView);
const tData = lView![TVIEW].data;
const tNode = tData[context.nodeIndex] as TNode;
return tNode.value!;
} catch (e) {
Expand All @@ -290,16 +292,17 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
return {};
}

const lView = context.lView;
const tData = lView[TVIEW].data;
const lView = getLViewById(context.lViewId);
ngDevMode && assertLView(lView);
const tData = lView![TVIEW].data;
const tNode = tData[context.nodeIndex] as TNode;

const properties: {[key: string]: string} = {};
// Collect properties from the DOM.
copyDomProperties(this.nativeElement, properties);
// Collect properties from the bindings. This is needed for animation renderer which has
// synthetic properties which don't get reflected into the DOM.
collectPropertyBindings(properties, tNode, lView, tData);
collectPropertyBindings(properties, tNode, lView!, tData);
return properties;
}

Expand All @@ -316,8 +319,9 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
return {};
}

const lView = context.lView;
const tNodeAttrs = (lView[TVIEW].data[context.nodeIndex] as TNode).attrs;
const lView = getLViewById(context.lViewId);
ngDevMode && assertLView(lView);
const tNodeAttrs = (lView![TVIEW].data[context.nodeIndex] as TNode).attrs;
const lowercaseTNodeAttrs: string[] = [];

// For debug nodes we take the element's attribute directly from the DOM since it allows us
Expand Down Expand Up @@ -503,9 +507,11 @@ function _queryAllR3(
matches: DebugElement[]|DebugNode[], elementsOnly: boolean) {
const context = loadLContext(parentElement.nativeNode, false);
if (context !== null) {
const parentTNode = context.lView[TVIEW].data[context.nodeIndex] as TNode;
const lView = getLViewById(context.lViewId);
ngDevMode && assertLView(lView);
const parentTNode = lView![TVIEW].data[context.nodeIndex] as TNode;
_queryNodeChildrenR3(
parentTNode, context.lView, predicate, matches, elementsOnly, parentElement.nativeNode);
parentTNode, lView!, predicate, matches, elementsOnly, parentElement.nativeNode);
} else {
// If the context is null, then `parentElement` was either created with Renderer2 or native DOM
// APIs.
Expand Down
38 changes: 24 additions & 14 deletions packages/core/src/render3/context_discovery.ts
Expand Up @@ -8,12 +8,14 @@
import '../util/ng_dev_mode';

import {assertDefined, assertDomNode} from '../util/assert';

import {EMPTY_ARRAY} from '../util/empty';

import {assertLView} from './assert';
import {getLViewById} from './instructions/lview_tracking';
import {LContext} from './interfaces/context';
import {TNode, TNodeFlags} from './interfaces/node';
import {RElement, RNode} from './interfaces/renderer_dom';
import {CONTEXT, HEADER_OFFSET, HOST, LView, TVIEW} from './interfaces/view';
import {CONTEXT, HEADER_OFFSET, HOST, ID, LView, TVIEW} from './interfaces/view';
import {getComponentLViewByIndex, unwrapRNode} from './util/view_utils';


Expand Down Expand Up @@ -109,7 +111,8 @@ export function getLContext(target: any): LContext|null {
if (Array.isArray(parentContext)) {
lView = parentContext as LView;
} else {
lView = parentContext.lView;
lView = getLViewById(parentContext.lViewId);
ngDevMode && assertLView(lView);
}

// the edge of the app was also reached here through another means
Expand Down Expand Up @@ -137,7 +140,7 @@ export function getLContext(target: any): LContext|null {
*/
function createLContext(lView: LView, nodeIndex: number, native: RNode): LContext {
return {
lView,
lViewId: lView[ID],
nodeIndex,
native,
component: undefined,
Expand All @@ -153,19 +156,21 @@ function createLContext(lView: LView, nodeIndex: number, native: RNode): LContex
* @returns The component's view
*/
export function getComponentViewByInstance(componentInstance: {}): LView {
let lView = readPatchedData(componentInstance);
let patchedData = readPatchedData(componentInstance);
let view: LView;

if (Array.isArray(lView)) {
const nodeIndex = findViaComponent(lView, componentInstance);
view = getComponentLViewByIndex(nodeIndex, lView);
const context = createLContext(lView, nodeIndex, view[HOST] as RElement);
if (Array.isArray(patchedData)) {
const nodeIndex = findViaComponent(patchedData, componentInstance);
view = getComponentLViewByIndex(nodeIndex, patchedData);
const context = createLContext(patchedData, nodeIndex, view[HOST] as RElement);
context.component = componentInstance;
attachPatchData(componentInstance, context);
attachPatchData(context.native, context);
} else {
const context = lView as any as LContext;
view = getComponentLViewByIndex(context.nodeIndex, context.lView);
const context = patchedData as any as LContext;
const lView = getLViewById(context.lViewId)!;
ngDevMode && assertLView(lView);
view = getComponentLViewByIndex(context.nodeIndex, lView);
}
return view;
}
Expand All @@ -181,7 +186,8 @@ const MONKEY_PATCH_KEY_NAME = '__ngContext__';
*/
export function attachPatchData(target: any, data: LView|LContext) {
ngDevMode && assertDefined(target, 'Target expected');
target[MONKEY_PATCH_KEY_NAME] = data;
// Only attach the ID of the view in order to avoid memory leaks (see #41047).
target[MONKEY_PATCH_KEY_NAME] = Array.isArray(data) ? data[ID] : data;
}

/**
Expand All @@ -190,13 +196,17 @@ export function attachPatchData(target: any, data: LView|LContext) {
*/
export function readPatchedData(target: any): LView|LContext|null {
ngDevMode && assertDefined(target, 'Target expected');
return target[MONKEY_PATCH_KEY_NAME] || null;
const data = target[MONKEY_PATCH_KEY_NAME];
if (typeof data === 'number') {
return getLViewById(data);
}
return data || null;
}

export function readPatchedLView(target: any): LView|null {
const value = readPatchedData(target);
if (value) {
return Array.isArray(value) ? value : (value as LContext).lView;
return Array.isArray(value) ? value : getLViewById(value.lViewId);
}
return null;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/render3/instructions/lview_debug.ts
Expand Up @@ -25,7 +25,7 @@ import {LQueries, TQueries} from '../interfaces/query';
import {Renderer3, RendererFactory3} from '../interfaces/renderer';
import {RComment, RElement, RNode} from '../interfaces/renderer_dom';
import {getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate, TStylingKey, TStylingRange} from '../interfaces/styling';
import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DebugNode, DECLARATION_VIEW, DestroyHookData, FLAGS, HEADER_OFFSET, HookData, HOST, HostBindingOpCodes, INJECTOR, LContainerDebug as ILContainerDebug, LView, LViewDebug as ILViewDebug, LViewDebugRange, LViewDebugRangeContent, LViewFlags, NEXT, NodeInjectorDebug, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, SANITIZER, T_HOST, TData, TView as ITView, TVIEW, TView, TViewType, TViewTypeAsString} from '../interfaces/view';
import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DebugNode, DECLARATION_VIEW, DestroyHookData, FLAGS, HEADER_OFFSET, HookData, HOST, HostBindingOpCodes, ID, INJECTOR, LContainerDebug as ILContainerDebug, LView, LViewDebug as ILViewDebug, LViewDebugRange, LViewDebugRangeContent, LViewFlags, NEXT, NodeInjectorDebug, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, SANITIZER, T_HOST, TData, TView as ITView, TVIEW, TView, TViewType, TViewTypeAsString} from '../interfaces/view';
import {attachDebugObject} from '../util/debug_utils';
import {getParentInjectorIndex, getParentInjectorView} from '../util/injector_utils';
import {unwrapRNode} from '../util/view_utils';
Expand Down Expand Up @@ -513,6 +513,9 @@ export class LViewDebug implements ILViewDebug {
get tHost(): ITNode|null {
return this._raw_lView[T_HOST];
}
get id(): number {
return this._raw_lView[ID];
}

get decls(): LViewDebugRange {
return toLViewRange(this.tView, this._raw_lView, HEADER_OFFSET, this.tView.bindingStartIndex);
Expand Down
35 changes: 35 additions & 0 deletions packages/core/src/render3/instructions/lview_tracking.ts
@@ -0,0 +1,35 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {assertNumber} from '../../util/assert';
import {ID, LView} from '../interfaces/view';

// Keeps track of the currently-active LViews.
const TRACKED_LVIEWS = new Map<number, LView>();

// Used for generating unique IDs for LViews.
let uniqueIdCounter = 0;

/** Starts tracking an LView and returns a unique ID that can be used for future lookups. */
export function trackLView(lView: LView): number {
const id = uniqueIdCounter++;
TRACKED_LVIEWS.set(id, lView);
return id;
}

/** Gets an LView by its unique ID. */
export function getLViewById(id: number): LView|null {
ngDevMode && assertNumber(id, 'ID used for LView lookup must be a number');
return TRACKED_LVIEWS.get(id) || null;
}

/** Stops tracking an LView. */
export function stopTrackingLView(lView: LView): void {
ngDevMode && assertNumber(lView[ID], 'Cannot stop tracking an LView that does not have an ID');
TRACKED_LVIEWS.delete(lView[ID]);
}
4 changes: 3 additions & 1 deletion packages/core/src/render3/instructions/shared.ts
Expand Up @@ -33,7 +33,7 @@ import {isProceduralRenderer, Renderer3, RendererFactory3} from '../interfaces/r
import {RComment, RElement, RNode, RText} from '../interfaces/renderer_dom';
import {SanitizerFn} from '../interfaces/sanitization';
import {isComponentDef, isComponentHost, isContentQueryHost, isRootView} from '../interfaces/type_checks';
import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HostBindingOpCodes, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TRANSPLANTED_VIEWS_TO_REFRESH, TVIEW, TView, TViewType} from '../interfaces/view';
import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HostBindingOpCodes, ID, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TRANSPLANTED_VIEWS_TO_REFRESH, TVIEW, TView, TViewType} from '../interfaces/view';
import {assertPureTNodeType, assertTNodeType} from '../node_assert';
import {updateTextNode} from '../node_manipulation';
import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher';
Expand All @@ -47,6 +47,7 @@ import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreation

import {selectIndexInternal} from './advance';
import {attachLContainerDebug, attachLViewDebug, cloneToLViewFromTViewBlueprint, cloneToTViewData, LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeDebug, TNodeInitialInputs, TNodeLocalNames, TViewComponents, TViewConstructor} from './lview_debug';
import {trackLView} from './lview_tracking';



Expand Down Expand Up @@ -142,6 +143,7 @@ export function createLView<T>(
lView[SANITIZER] = sanitizer || parentLView && parentLView[SANITIZER] || null!;
lView[INJECTOR as any] = injector || parentLView && parentLView[INJECTOR] || null;
lView[T_HOST] = tHostNode;
lView[ID] = trackLView(lView);
ngDevMode &&
assertEqual(
tView.type == TViewType.Embedded ? parentLView !== null : true, true,
Expand Down
5 changes: 2 additions & 3 deletions packages/core/src/render3/interfaces/context.ts
Expand Up @@ -8,7 +8,6 @@


import {RNode} from './renderer_dom';
import {LView} from './view';


/**
Expand All @@ -23,9 +22,9 @@ import {LView} from './view';
*/
export interface LContext {
/**
* The component's parent view data.
* ID of the component's parent view data.
*/
lView: LView;
lViewId: number;

/**
* The index instance of the node.
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/render3/interfaces/view.ts
Expand Up @@ -47,14 +47,15 @@ export const DECLARATION_COMPONENT_VIEW = 16;
export const DECLARATION_LCONTAINER = 17;
export const PREORDER_HOOK_FLAGS = 18;
export const QUERIES = 19;
export const ID = 20;
/**
* Size of LView's header. Necessary to adjust for it when setting slots.
*
* IMPORTANT: `HEADER_OFFSET` should only be referred to the in the `ɵɵ*` instructions to translate
* instruction index into `LView` index. All other indexes should be in the `LView` index space and
* there should be no need to refer to `HEADER_OFFSET` anywhere else.
*/
export const HEADER_OFFSET = 20;
export const HEADER_OFFSET = 21;


// This interface replaces the real LView interface if it is an arg or a
Expand Down Expand Up @@ -326,6 +327,9 @@ export interface LView extends Array<any> {
* are not `Dirty`/`CheckAlways`.
*/
[TRANSPLANTED_VIEWS_TO_REFRESH]: number;

/** Unique ID of the view. Used for `__ngContext__` lookups in the `LView` registry. */
[ID]: number;
}

/** Flags associated with an LView (saved in LView[FLAGS]) */
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/render3/node_manipulation.ts
Expand Up @@ -16,6 +16,7 @@ import {escapeCommentText} from '../util/dom';
import {assertLContainer, assertLView, assertParentView, assertProjectionSlots, assertTNodeForLView} from './assert';
import {attachPatchData} from './context_discovery';
import {icuContainerIterate} from './i18n/i18n_tree_shaking';
import {stopTrackingLView} from './instructions/lview_tracking';
import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
import {ComponentDef} from './interfaces/definition';
import {NodeInjectorFactory} from './interfaces/injector';
Expand Down Expand Up @@ -395,6 +396,7 @@ export function destroyLView(tView: TView, lView: LView) {
}

destroyViewTree(lView);
stopTrackingLView(lView);
}
}

Expand Down

0 comments on commit 7038e66

Please sign in to comment.