Skip to content

Commit

Permalink
Revert "perf(core): avoid storing LView in __ngContext__ (#41358)" (#…
Browse files Browse the repository at this point in the history
…41901)

This reverts commit 18b33e7.

PR Close #41901
  • Loading branch information
crisbeto authored and mhevery committed Apr 30, 2021
1 parent be9b19e commit 1c0db8c
Show file tree
Hide file tree
Showing 32 changed files with 171 additions and 425 deletions.
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -3,7 +3,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1170,
"main-es2015": 138718,
"main-es2015": 138189,
"polyfills-es2015": 36964
}
}
Expand All @@ -30,7 +30,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1190,
"main-es2015": 137087,
"main-es2015": 136546,
"polyfills-es2015": 37641
}
}
Expand Down
30 changes: 13 additions & 17 deletions packages/core/src/debug/debug_node.ts
Expand Up @@ -262,10 +262,9 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
}

get name(): string {
const context = getLContext(this.nativeNode)!;
const lView = context ? context.lView : null;

if (lView !== null) {
const context = getLContext(this.nativeNode);
if (context !== null) {
const lView = context.lView;
const tData = lView[TVIEW].data;
const tNode = tData[context.nodeIndex] as TNode;
return tNode.value!;
Expand All @@ -287,13 +286,12 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
* - attribute bindings (e.g. `[attr.role]="menu"`)
*/
get properties(): {[key: string]: any;} {
const context = getLContext(this.nativeNode)!;
const lView = context ? context.lView : null;

if (lView === null) {
const context = getLContext(this.nativeNode);
if (context === null) {
return {};
}

const lView = context.lView;
const tData = lView[TVIEW].data;
const tNode = tData[context.nodeIndex] as TNode;

Expand All @@ -314,13 +312,12 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
return attributes;
}

const context = getLContext(element)!;
const lView = context ? context.lView : null;

if (lView === null) {
const context = getLContext(element);
if (context === null) {
return {};
}

const lView = context.lView;
const tNodeAttrs = (lView[TVIEW].data[context.nodeIndex] as TNode).attrs;
const lowercaseTNodeAttrs: string[] = [];

Expand Down Expand Up @@ -505,12 +502,11 @@ function _queryAllR3(
function _queryAllR3(
parentElement: DebugElement, predicate: Predicate<DebugElement>|Predicate<DebugNode>,
matches: DebugElement[]|DebugNode[], elementsOnly: boolean) {
const context = getLContext(parentElement.nativeNode)!;
const lView = context ? context.lView : null;
if (lView !== null) {
const parentTNode = lView[TVIEW].data[context.nodeIndex] as TNode;
const context = getLContext(parentElement.nativeNode);
if (context !== null) {
const parentTNode = context.lView[TVIEW].data[context.nodeIndex] as TNode;
_queryNodeChildrenR3(
parentTNode, lView, predicate, matches, elementsOnly, parentElement.nativeNode);
parentTNode, context.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
56 changes: 29 additions & 27 deletions packages/core/src/render3/context_discovery.ts
Expand Up @@ -8,15 +8,12 @@
import '../util/ng_dev_mode';

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

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


Expand Down Expand Up @@ -46,7 +43,7 @@ export function getLContext(target: any): LContext|null {
if (mpValue) {
// only when it's an array is it considered an LView instance
// ... otherwise it's an already constructed LContext instance
if (isLView(mpValue)) {
if (Array.isArray(mpValue)) {
const lView: LView = mpValue!;
let nodeIndex: number;
let component: any = undefined;
Expand Down Expand Up @@ -108,7 +105,12 @@ export function getLContext(target: any): LContext|null {
while (parent = parent.parentNode) {
const parentContext = readPatchedData(parent);
if (parentContext) {
const lView = Array.isArray(parentContext) ? parentContext as LView : parentContext.lView;
let lView: LView|null;
if (Array.isArray(parentContext)) {
lView = parentContext as LView;
} else {
lView = parentContext.lView;
}

// the edge of the app was also reached here through another means
// (maybe because the DOM was changed manually).
Expand All @@ -134,7 +136,14 @@ export function getLContext(target: any): LContext|null {
* Creates an empty instance of a `LContext` context
*/
function createLContext(lView: LView, nodeIndex: number, native: RNode): LContext {
return new LContext(lView[ID], nodeIndex, native);
return {
lView,
nodeIndex,
native,
component: undefined,
directives: undefined,
localRefs: undefined,
};
}

/**
Expand All @@ -144,24 +153,21 @@ function createLContext(lView: LView, nodeIndex: number, native: RNode): LContex
* @returns The component's view
*/
export function getComponentViewByInstance(componentInstance: {}): LView {
let patchedData = readPatchedData(componentInstance);
let lView: LView;
let lView = readPatchedData(componentInstance);
let view: LView;

if (isLView(patchedData)) {
const contextLView: LView = patchedData;
const nodeIndex = findViaComponent(contextLView, componentInstance);
lView = getComponentLViewByIndex(nodeIndex, contextLView);
const context = createLContext(contextLView, nodeIndex, lView[HOST] as RElement);
if (Array.isArray(lView)) {
const nodeIndex = findViaComponent(lView, componentInstance);
view = getComponentLViewByIndex(nodeIndex, lView);
const context = createLContext(lView, nodeIndex, view[HOST] as RElement);
context.component = componentInstance;
attachPatchData(componentInstance, context);
attachPatchData(context.native, context);
} else {
const context = patchedData as unknown as LContext;
const contextLView = context.lView!;
ngDevMode && assertLView(contextLView);
lView = getComponentLViewByIndex(context.nodeIndex, contextLView);
const context = lView as any as LContext;
view = getComponentLViewByIndex(context.nodeIndex, context.lView);
}
return lView;
return view;
}

/**
Expand All @@ -175,10 +181,7 @@ 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). 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;
target[MONKEY_PATCH_KEY_NAME] = data;
}

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

export function readPatchedLView(target: any): LView|null {
const value = readPatchedData(target);
if (value) {
return isLView(value) ? value : value.lView;
return Array.isArray(value) ? value : (value as LContext).lView;
}
return null;
}
Expand Down
5 changes: 1 addition & 4 deletions 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, 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 {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 {attachDebugObject} from '../util/debug_utils';
import {getParentInjectorIndex, getParentInjectorView} from '../util/injector_utils';
import {unwrapRNode} from '../util/view_utils';
Expand Down Expand Up @@ -513,9 +513,6 @@ 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
13 changes: 4 additions & 9 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -28,13 +28,12 @@ import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} fr
import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container';
import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, HostBindingsFunction, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
import {NodeInjectorFactory} from '../interfaces/injector';
import {registerLView} from '../interfaces/lview_tracking';
import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliases, PropertyAliasValue, TAttributes, TConstantsOrFactory, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeType, TProjectionNode} from '../interfaces/node';
import {isProceduralRenderer, Renderer3, RendererFactory3} from '../interfaces/renderer';
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, 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 {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 {assertPureTNodeType, assertTNodeType} from '../node_assert';
import {updateTextNode} from '../node_manipulation';
import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher';
Expand Down Expand Up @@ -144,7 +143,6 @@ 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] = registerLView(lView);
ngDevMode &&
assertEqual(
tView.type == TViewType.Embedded ? parentLView !== null : true, true,
Expand Down Expand Up @@ -1908,12 +1906,9 @@ export function scheduleTick(rootContext: RootContext, flags: RootContextFlags)
export function tickRootContext(rootContext: RootContext) {
for (let i = 0; i < rootContext.components.length; i++) {
const rootComponent = rootContext.components[i];
const lView = readPatchedLView(rootComponent);
// We might not have an `LView` if the component was destroyed.
if (lView !== null) {
const tView = lView[TVIEW];
renderComponentOrTemplate(tView, lView, tView.template, rootComponent);
}
const lView = readPatchedLView(rootComponent)!;
const tView = lView[TVIEW];
renderComponentOrTemplate(tView, lView, tView.template, rootComponent);
}
}

Expand Down
49 changes: 21 additions & 28 deletions packages/core/src/render3/interfaces/context.ts
Expand Up @@ -7,7 +7,6 @@
*/


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

Expand All @@ -22,41 +21,35 @@ import {LView} from './view';
* function. The component, element and each directive instance will share the same instance
* of the context.
*/
export class LContext {
export interface LContext {
/**
* The component's parent view data.
*/
lView: LView;

/**
* The index instance of the node.
*/
nodeIndex: number;

/**
* The instance of the DOM node that is attached to the lNode.
*/
native: RNode;

/**
* The instance of the Component node.
*/
public component: {}|null|undefined;
component: {}|null|undefined;

/**
* The list of active directives that exist on this element.
*/
public directives: any[]|null|undefined;
directives: any[]|null|undefined;

/**
* The map of local references (local reference name => element or directive instance) that
* exist on this element.
* The map of local references (local reference name => element or directive instance) that exist
* on this element.
*/
public localRefs: {[key: string]: any}|null|undefined;

/** Component's parent view data. */
get lView(): LView|null {
return getLViewById(this.lViewId);
}

constructor(
/**
* ID of the component's parent view data.
*/
private lViewId: number,

/**
* The index instance of the node.
*/
public nodeIndex: number,

/**
* The instance of the DOM node that is attached to the lNode.
*/
public native: RNode) {}
localRefs: {[key: string]: any}|null|undefined;
}
35 changes: 0 additions & 35 deletions packages/core/src/render3/interfaces/lview_tracking.ts

This file was deleted.

6 changes: 1 addition & 5 deletions packages/core/src/render3/interfaces/view.ts
Expand Up @@ -47,15 +47,14 @@ 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 = 21;
export const HEADER_OFFSET = 20;


// This interface replaces the real LView interface if it is an arg or a
Expand Down Expand Up @@ -327,9 +326,6 @@ 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

0 comments on commit 1c0db8c

Please sign in to comment.