Skip to content

Commit

Permalink
fixup! perf(core): avoid storing LView in __ngContext__
Browse files Browse the repository at this point in the history
  • Loading branch information
crisbeto committed Mar 30, 2021
1 parent 78d877e commit 186f79d
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 24 deletions.
6 changes: 3 additions & 3 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -3,7 +3,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 138189,
"main-es2015": 138807,
"polyfills-es2015": 36964
}
}
Expand All @@ -30,7 +30,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 136546,
"main-es2015": 137148,
"polyfills-es2015": 37641
}
}
Expand All @@ -49,7 +49,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 216267,
"main-es2015": 216898,
"polyfills-es2015": 36723,
"5-es2015": 781
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/debug/debug_node.ts
Expand Up @@ -292,17 +292,17 @@ 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} = {};
// 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 Down
18 changes: 10 additions & 8 deletions packages/core/src/render3/context_discovery.ts
Expand Up @@ -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;
}

/**
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/instructions/lview_tracking.ts
Expand Up @@ -16,7 +16,7 @@ const TRACKED_LVIEWS = new Map<number, LView>();
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;
Expand All @@ -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]);
}
4 changes: 2 additions & 2 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -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';



Expand Down Expand Up @@ -143,7 +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);
lView[ID] = registerLView(lView);
ngDevMode &&
assertEqual(
tView.type == TViewType.Embedded ? parentLView !== null : true, true,
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/node_manipulation.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -396,7 +396,7 @@ export function destroyLView(tView: TView, lView: LView) {
}

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

Expand Down
12 changes: 8 additions & 4 deletions packages/core/src/render3/util/discovery_utils.ts
Expand Up @@ -50,7 +50,7 @@ import {getTNode, unwrapRNode} from './view_utils';
* @globalApi ng
*/
export function getComponent<T>(element: Element): T|null {
assertDomElement(element);
ngDevMode && assertDomElement(element);
const context = loadLContext(element, false);
if (context === null) return null;

Expand Down Expand Up @@ -79,7 +79,11 @@ export function getComponent<T>(element: Element): T|null {
export function getContext<T>(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;
}

Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit 186f79d

Please sign in to comment.