From 186f79d832ba46ac9346b5632aa657a688e982b3 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 30 Mar 2021 13:04:49 +0200 Subject: [PATCH] fixup! perf(core): avoid storing LView in __ngContext__ --- .../size-tracking/integration-payloads.json | 6 +++--- packages/core/src/debug/debug_node.ts | 6 +++--- packages/core/src/render3/context_discovery.ts | 18 ++++++++++-------- .../src/render3/instructions/lview_tracking.ts | 4 ++-- .../core/src/render3/instructions/shared.ts | 4 ++-- packages/core/src/render3/node_manipulation.ts | 4 ++-- .../core/src/render3/util/discovery_utils.ts | 12 ++++++++---- 7 files changed, 30 insertions(+), 24 deletions(-) diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 1410f8ab36349..8f41bc63b2601 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -3,7 +3,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 138189, + "main-es2015": 138807, "polyfills-es2015": 36964 } } @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 136546, + "main-es2015": 137148, "polyfills-es2015": 37641 } } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 216267, + "main-es2015": 216898, "polyfills-es2015": 36723, "5-es2015": 781 } diff --git a/packages/core/src/debug/debug_node.ts b/packages/core/src/debug/debug_node.ts index 6993c92af56fe..d5ab09aafb627 100644 --- a/packages/core/src/debug/debug_node.ts +++ b/packages/core/src/debug/debug_node.ts @@ -292,9 +292,9 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme return {}; } - const lView = getLViewById(context.lViewId); + const lView = getLViewById(context.lViewId)!; ngDevMode && assertLView(lView); - const tData = lView![TVIEW].data; + const tData = lView[TVIEW].data; const tNode = tData[context.nodeIndex] as TNode; const properties: {[key: string]: string} = {}; @@ -302,7 +302,7 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme 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; } diff --git a/packages/core/src/render3/context_discovery.ts b/packages/core/src/render3/context_discovery.ts index e71925db7d49c..0585ae460800a 100644 --- a/packages/core/src/render3/context_discovery.ts +++ b/packages/core/src/render3/context_discovery.ts @@ -158,22 +158,22 @@ function createLContext(lView: LView, nodeIndex: number, native: RNode): LContex */ export function getComponentViewByInstance(componentInstance: {}): LView { let patchedData = readPatchedData(componentInstance); - let view: LView; + let lView: LView; if (isLView(patchedData)) { const nodeIndex = findViaComponent(patchedData, componentInstance); - view = getComponentLViewByIndex(nodeIndex, patchedData); - const context = createLContext(patchedData, nodeIndex, view[HOST] as RElement); + lView = getComponentLViewByIndex(nodeIndex, patchedData); + const context = createLContext(patchedData, nodeIndex, lView[HOST] as RElement); context.component = componentInstance; attachPatchData(componentInstance, context); attachPatchData(context.native, context); } else { const context = patchedData as any as LContext; - const lView = getLViewById(context.lViewId)!; - ngDevMode && assertLView(lView); - view = getComponentLViewByIndex(context.nodeIndex, lView); + const contextLView = getLViewById(context.lViewId)!; + ngDevMode && assertLView(contextLView); + lView = getComponentLViewByIndex(context.nodeIndex, contextLView); } - return view; + return lView; } /** @@ -187,7 +187,9 @@ const MONKEY_PATCH_KEY_NAME = '__ngContext__'; */ export function attachPatchData(target: any, data: LView|LContext) { ngDevMode && assertDefined(target, 'Target expected'); - // Only attach the ID of the view in order to avoid memory leaks (see #41047). + // Only attach the ID of the view in order to avoid memory leaks (see #41047). We only do this + // for `LView`, because we have control over when an `LView` is created and destroyed, whereas + // we can't know when to remove an `LContext`. target[MONKEY_PATCH_KEY_NAME] = isLView(data) ? data[ID] : data; } diff --git a/packages/core/src/render3/instructions/lview_tracking.ts b/packages/core/src/render3/instructions/lview_tracking.ts index 290b7ebddc817..326990d1a417e 100644 --- a/packages/core/src/render3/instructions/lview_tracking.ts +++ b/packages/core/src/render3/instructions/lview_tracking.ts @@ -16,7 +16,7 @@ const TRACKED_LVIEWS = new Map(); 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 { +export function registerLView(lView: LView): number { const id = uniqueIdCounter++; TRACKED_LVIEWS.set(id, lView); return id; @@ -29,7 +29,7 @@ export function getLViewById(id: number): LView|null { } /** Stops tracking an LView. */ -export function stopTrackingLView(lView: LView): void { +export function unregisterLView(lView: LView): void { ngDevMode && assertNumber(lView[ID], 'Cannot stop tracking an LView that does not have an ID'); TRACKED_LVIEWS.delete(lView[ID]); } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index d980ce647cc57..8b3a588d02c38 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -47,7 +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'; +import {registerLView} from './lview_tracking'; @@ -143,7 +143,7 @@ export function createLView( lView[SANITIZER] = sanitizer || parentLView && parentLView[SANITIZER] || null!; lView[INJECTOR as any] = injector || parentLView && parentLView[INJECTOR] || null; lView[T_HOST] = tHostNode; - lView[ID] = trackLView(lView); + lView[ID] = registerLView(lView); ngDevMode && assertEqual( tView.type == TViewType.Embedded ? parentLView !== null : true, true, diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index f10ae8042c1fd..b1b1172f0628e 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -16,7 +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 {unregisterLView} 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'; @@ -396,7 +396,7 @@ export function destroyLView(tView: TView, lView: LView) { } destroyViewTree(lView); - stopTrackingLView(lView); + unregisterLView(lView); } } diff --git a/packages/core/src/render3/util/discovery_utils.ts b/packages/core/src/render3/util/discovery_utils.ts index 1e4fa5af4e98e..26eb9c7abfc11 100644 --- a/packages/core/src/render3/util/discovery_utils.ts +++ b/packages/core/src/render3/util/discovery_utils.ts @@ -50,7 +50,7 @@ import {getTNode, unwrapRNode} from './view_utils'; * @globalApi ng */ export function getComponent(element: Element): T|null { - assertDomElement(element); + ngDevMode && assertDomElement(element); const context = loadLContext(element, false); if (context === null) return null; @@ -79,7 +79,11 @@ export function getComponent(element: Element): T|null { export function getContext(element: Element): T|null { assertDomElement(element); const context = loadLContext(element, false); - const lView = context === null ? null : getLViewById(context.lViewId)!; + let lView: LView|null = null; + if (context !== null) { + lView = getLViewById(context.lViewId); + ngDevMode && assertLView(lView); + } return lView === null ? null : lView[CONTEXT] as T; } @@ -335,9 +339,9 @@ export interface Listener { * @globalApi ng */ export function getListeners(element: Element): Listener[] { - assertDomElement(element); + ngDevMode && assertDomElement(element); const lContext = loadLContext(element, false); - const lView = lContext ? getLViewById(lContext.lViewId) : null; + const lView = lContext === null ? null : getLViewById(lContext.lViewId); if (lView === null) return []; const tView = lView[TVIEW];