Skip to content

Commit

Permalink
perf(ivy): store views directly on LContainer
Browse files Browse the repository at this point in the history
Stores the views that are part of a container directly on the `LContainer`, rather than maintaining a dedicated sub-array.

This PR resolves FW-1288.
  • Loading branch information
crisbeto committed Apr 29, 2019
1 parent 537502d commit 04f8af9
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 84 deletions.
8 changes: 4 additions & 4 deletions packages/core/src/debug/debug_node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

import {Injector} from '../di';
import {getViewComponent} from '../render3/global_utils_api';
import {LContainer, NATIVE, VIEWS} from '../render3/interfaces/container';
import {CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from '../render3/interfaces/container';
import {TElementNode, TNode, TNodeFlags, TNodeType} from '../render3/interfaces/node';
import {StylingIndex} from '../render3/interfaces/styling';
import {LView, NEXT, PARENT, TData, TVIEW, T_HOST} from '../render3/interfaces/view';
import {LView, PARENT, TData, TVIEW, T_HOST} from '../render3/interfaces/view';
import {getProp, getValue, isClassBasedValue} from '../render3/styling/class_and_style_bindings';
import {getStylingContextFromLView} from '../render3/styling/util';
import {getComponent, getContext, getInjectionTokens, getInjector, getListeners, getLocalRefs, isBrowserEvents, loadLContext, loadLContextFromNode} from '../render3/util/discovery_utils';
Expand Down Expand Up @@ -502,8 +502,8 @@ function _queryNodeChildrenR3(
function _queryNodeChildrenInContainerR3(
lContainer: LContainer, predicate: Predicate<DebugNode>, matches: DebugNode[],
elementsOnly: boolean, rootNativeNode: any) {
for (let i = 0; i < lContainer[VIEWS].length; i++) {
const childView = lContainer[VIEWS][i];
for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
const childView = lContainer[i];
_queryNodeChildrenR3(
childView[TVIEW].node !, childView, predicate, matches, elementsOnly, rootNativeNode);
}
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/render3/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
*/

import {assertDefined} from '../util/assert';
import {ACTIVE_INDEX, LContainer, NATIVE, VIEWS} from './interfaces/container';

import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from './interfaces/container';
import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, TIcu} from './interfaces/i18n';
import {TNode} from './interfaces/node';
import {LQueries} from './interfaces/query';
Expand Down Expand Up @@ -207,7 +208,8 @@ export class LContainerDebug {

get activeIndex(): number { return this._raw_lContainer[ACTIVE_INDEX]; }
get views(): LViewDebug[] {
return this._raw_lContainer[VIEWS].map(toDebug as(l: LView) => LViewDebug);
return this._raw_lContainer.slice(CONTAINER_HEADER_OFFSET)
.map(toDebug as(l: LView) => LViewDebug);
}
get parent(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lContainer[PARENT]); }
get queries(): LQueries|null { return this._raw_lContainer[QUERIES]; }
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/render3/instructions/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {assertEqual} from '../../util/assert';
import {assertHasParent} from '../assert';
import {attachPatchData} from '../context_discovery';
import {executePreOrderHooks, registerPostOrderHooks} from '../hooks';
import {ACTIVE_INDEX, VIEWS} from '../interfaces/container';
import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
import {ComponentTemplate} from '../interfaces/definition';
import {LocalRefExtractor, TAttributes, TContainerNode, TNode, TNodeType} from '../interfaces/node';
import {BINDING_INDEX, HEADER_OFFSET, LView, QUERIES, RENDERER, TVIEW} from '../interfaces/view';
Expand Down Expand Up @@ -123,11 +123,11 @@ export function ɵɵcontainerRefreshEnd(): void {

ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.Container);

const lContainer = getLView()[previousOrParentTNode.index];
const lContainer: LContainer = getLView()[previousOrParentTNode.index];
const nextIndex = lContainer[ACTIVE_INDEX];

// remove extra views at the end of the container
while (nextIndex < lContainer[VIEWS].length) {
while (nextIndex < lContainer.length - CONTAINER_HEADER_OFFSET) {
removeView(lContainer, nextIndex);
}
}
Expand Down
12 changes: 5 additions & 7 deletions packages/core/src/render3/instructions/embedded_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {assertDefined, assertEqual} from '../../util/assert';
import {assertLContainerOrUndefined} from '../assert';
import {ACTIVE_INDEX, LContainer, VIEWS} from '../interfaces/container';
import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
import {RenderFlags} from '../interfaces/definition';
import {TContainerNode, TNodeType} from '../interfaces/node';
import {FLAGS, LView, LViewFlags, PARENT, QUERIES, TVIEW, TView, T_HOST} from '../interfaces/view';
Expand Down Expand Up @@ -104,17 +104,15 @@ function getOrCreateEmbeddedTView(
* @param lContainer to search for views
* @param startIdx starting index in the views array to search from
* @param viewBlockId exact view block id to look for
* @returns index of a found view or -1 if not found
*/
function scanForView(lContainer: LContainer, startIdx: number, viewBlockId: number): LView|null {
const views = lContainer[VIEWS];
for (let i = startIdx; i < views.length; i++) {
const viewAtPositionId = views[i][TVIEW].id;
for (let i = startIdx + CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
const viewAtPositionId = lContainer[i][TVIEW].id;
if (viewAtPositionId === viewBlockId) {
return views[i];
return lContainer[i];
} else if (viewAtPositionId < viewBlockId) {
// found a view that should not be at this position - remove
removeView(lContainer, i);
removeView(lContainer, i - CONTAINER_HEADER_OFFSET);
} else {
// found a view with id greater than the one we are searching for
// which means that required view doesn't exist and can't be found at
Expand Down
12 changes: 5 additions & 7 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {attachLContainerDebug, attachLViewDebug} from '../debug';
import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di';
import {throwMultipleComponentError} from '../errors';
import {executeHooks, executePreOrderHooks, registerPreOrderHooks} from '../hooks';
import {ACTIVE_INDEX, LContainer, VIEWS} from '../interfaces/container';
import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from '../interfaces/injector';
import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TContainerNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
Expand All @@ -37,7 +37,7 @@ import {NO_CHANGE} from '../tokens';
import {attrsStylingIndexOf} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER, stringifyForError} from '../util/misc_utils';
import {getLViewParent, getRootContext} from '../util/view_traversal_utils';
import {getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isComponent, isComponentDef, isContentQueryHost, isRootView, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';
import {getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isComponent, isComponentDef, isContentQueryHost, isLContainer, isRootView, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';



Expand Down Expand Up @@ -1436,7 +1436,6 @@ export function createLContainer(
null, // queries
tNode, // t_host
native, // native
[], // views
];
ngDevMode && attachLContainerDebug(lContainer);
return lContainer;
Expand All @@ -1452,10 +1451,9 @@ function refreshDynamicEmbeddedViews(lView: LView) {
// Note: current can be an LView or an LContainer instance, but here we are only interested
// in LContainer. We can tell it's an LContainer because its length is less than the LView
// header.
if (current.length < HEADER_OFFSET && current[ACTIVE_INDEX] === -1) {
const container = current as LContainer;
for (let i = 0; i < container[VIEWS].length; i++) {
const dynamicViewData = container[VIEWS][i];
if (current[ACTIVE_INDEX] === -1 && isLContainer(current)) {
for (let i = CONTAINER_HEADER_OFFSET; i < current.length; i++) {
const dynamicViewData = current[i];
// The directives and pipes are not needed here as an existing view is only being refreshed.
ngDevMode && assertDefined(dynamicViewData[TVIEW], 'TView must be allocated');
renderEmbeddedTemplate(dynamicViewData, dynamicViewData[TVIEW], dynamicViewData[CONTEXT] !);
Expand Down
18 changes: 8 additions & 10 deletions packages/core/src/render3/interfaces/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ export const ACTIVE_INDEX = 2;
// PARENT, NEXT, QUERIES and T_HOST are indices 3, 4, 5 and 6.
// As we already have these constants in LView, we don't need to re-create them.
export const NATIVE = 7;
export const VIEWS = 8;

/**
* Size of LContainer's header. Represents the index after which all views in the
* container will be inserted. We need to keep a record of current views so we know
* which views are already in the DOM (and don't need to be re-added) and so we can
* remove views from the DOM when they are no longer required.
*/
export const CONTAINER_HEADER_OFFSET = 8;

/**
* The state associated with a container.
Expand Down Expand Up @@ -92,15 +99,6 @@ export interface LContainer extends Array<any> {
/** The comment element that serves as an anchor for this LContainer. */
readonly[NATIVE]:
RComment; // TODO(misko): remove as this value can be gotten by unwrapping `[HOST]`

/**
*A list of the container's currently active child views. Views will be inserted
*here as they are added and spliced from here when they are removed. We need
*to keep a record of current views so we know which views are already in the DOM
*(and don't need to be re-added) and so we can remove views from the DOM when they
*are no longer required.
*/
[VIEWS]: LView[];
}

// Note: This hack is necessary so we don't erroneously get a circular dependency
Expand Down
69 changes: 34 additions & 35 deletions packages/core/src/render3/node_manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@
*/

import {ViewEncapsulation} from '../metadata/view';
import {assertDefined} from '../util/assert';

import {assertLContainer, assertLView} from './assert';
import {attachPatchData} from './context_discovery';
import {LContainer, NATIVE, VIEWS, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
import {CONTAINER_HEADER_OFFSET, LContainer, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
import {ComponentDef} from './interfaces/definition';
import {NodeInjectorFactory} from './interfaces/injector';
import {TElementNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
Expand Down Expand Up @@ -99,8 +98,9 @@ function walkTNodeTree(
// This element has an LContainer, and its comment needs to be handled
executeNodeAction(
action, renderer, renderParent, nodeOrContainer[NATIVE], tNode, beforeNode);
if (nodeOrContainer[VIEWS].length) {
currentView = nodeOrContainer[VIEWS][0];
const firstView = nodeOrContainer[CONTAINER_HEADER_OFFSET];
if (firstView) {
currentView = firstView;
nextTNode = currentView[TVIEW].node;

// When the walker enters a container, then the beforeNode has to become the local native
Expand All @@ -111,9 +111,9 @@ function walkTNodeTree(
} else if (tNode.type === TNodeType.Container) {
const lContainer = currentView ![tNode.index] as LContainer;
executeNodeAction(action, renderer, renderParent, lContainer[NATIVE], tNode, beforeNode);

if (lContainer[VIEWS].length) {
currentView = lContainer[VIEWS][0];
const firstView = lContainer[CONTAINER_HEADER_OFFSET];
if (firstView) {
currentView = firstView;
nextTNode = currentView[TVIEW].node;

// When the walker enters a container, then the beforeNode has to become the local native
Expand Down Expand Up @@ -302,8 +302,8 @@ export function destroyViewTree(rootView: LView): void {
} else {
ngDevMode && assertLContainer(lViewOrLContainer);
// If container, traverse down to its first LView.
const views = lViewOrLContainer[VIEWS] as LView[];
if (views.length > 0) next = views[0];
const firstView: LView|undefined = lViewOrLContainer[CONTAINER_HEADER_OFFSET];
if (firstView) next = firstView;
}

if (!next) {
Expand Down Expand Up @@ -335,18 +335,18 @@ export function destroyViewTree(rootView: LView): void {
export function insertView(lView: LView, lContainer: LContainer, index: number) {
ngDevMode && assertLView(lView);
ngDevMode && assertLContainer(lContainer);
const views = lContainer[VIEWS];
ngDevMode && assertDefined(views, 'Container must have views');
const indexInContainer = CONTAINER_HEADER_OFFSET + index;
const containerLength = lContainer.length;

if (index > 0) {
// This is a new view, we need to add it to the children.
views[index - 1][NEXT] = lView;
lContainer[indexInContainer - 1][NEXT] = lView;
}

if (index < views.length) {
lView[NEXT] = views[index];
views.splice(index, 0, lView);
if (index < containerLength - CONTAINER_HEADER_OFFSET) {
lView[NEXT] = lContainer[indexInContainer];
lContainer.splice(CONTAINER_HEADER_OFFSET + index, 0, lView);
} else {
views.push(lView);
lContainer.push(lView);
lView[NEXT] = null;
}

Expand All @@ -372,13 +372,15 @@ export function insertView(lView: LView, lContainer: LContainer, index: number)
* @returns Detached LView instance.
*/
export function detachView(lContainer: LContainer, removeIndex: number): LView|undefined {
const views = lContainer[VIEWS];
const viewToDetach = views[removeIndex];
if (lContainer.length <= CONTAINER_HEADER_OFFSET) return;

const indexInContainer = CONTAINER_HEADER_OFFSET + removeIndex;
const viewToDetach = lContainer[indexInContainer];
if (viewToDetach) {
if (removeIndex > 0) {
views[removeIndex - 1][NEXT] = viewToDetach[NEXT] as LView;
lContainer[indexInContainer - 1][NEXT] = viewToDetach[NEXT] as LView;
}
views.splice(removeIndex, 1);
lContainer.splice(CONTAINER_HEADER_OFFSET + removeIndex, 1);
addRemoveViewFromContainer(viewToDetach, false);

if ((viewToDetach[FLAGS] & LViewFlags.Attached) &&
Expand All @@ -400,11 +402,8 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView|u
* @param removeIndex The index of the view to remove
*/
export function removeView(lContainer: LContainer, removeIndex: number) {
const view = lContainer[VIEWS][removeIndex];
if (view) {
detachView(lContainer, removeIndex);
destroyLView(view);
}
const detachedView = detachView(lContainer, removeIndex);
detachedView && destroyLView(detachedView);
}

/**
Expand Down Expand Up @@ -678,9 +677,8 @@ export function nativeNextSibling(renderer: Renderer3, node: RNode): RNode|null
function getNativeAnchorNode(parentTNode: TNode, lView: LView): RNode|null {
if (parentTNode.type === TNodeType.View) {
const lContainer = getLContainer(parentTNode as TViewNode, lView) !;
const views = lContainer[VIEWS];
const index = views.indexOf(lView);
return getBeforeNodeForView(index, views, lContainer[NATIVE]);
const index = lContainer.indexOf(lView, CONTAINER_HEADER_OFFSET) - CONTAINER_HEADER_OFFSET;
return getBeforeNodeForView(index, lContainer);
} else if (
parentTNode.type === TNodeType.ElementContainer ||
parentTNode.type === TNodeType.IcuContainer) {
Expand Down Expand Up @@ -729,9 +727,11 @@ function getHighestElementOrICUContainer(tNode: TNode): TNode {
return tNode;
}

export function getBeforeNodeForView(index: number, views: LView[], containerNative: RComment) {
if (index + 1 < views.length) {
const view = views[index + 1] as LView;
export function getBeforeNodeForView(index: number, lContainer: LContainer) {
const containerNative = lContainer[NATIVE];

if (index + 1 < lContainer.length - CONTAINER_HEADER_OFFSET) {
const view = lContainer[CONTAINER_HEADER_OFFSET + index + 1] as LView;
const viewTNode = view[T_HOST] as TViewNode;
return viewTNode.child ? getNativeByTNode(viewTNode.child, view) : containerNative;
} else {
Expand Down Expand Up @@ -817,9 +817,8 @@ function appendProjectedNode(
// Alternatively a container is projected at the root of a component's template
// and can't be re-projected (as not content of any component).
// Assign the final projection location in those cases.
const views = nodeOrContainer[VIEWS];
for (let i = 0; i < views.length; i++) {
addRemoveViewFromContainer(views[i], true, nodeOrContainer[NATIVE]);
for (let i = CONTAINER_HEADER_OFFSET; i < nodeOrContainer.length; i++) {
addRemoveViewFromContainer(nodeOrContainer[i], true, nodeOrContainer[NATIVE]);
}
} else {
if (projectedTNode.type === TNodeType.ElementContainer) {
Expand Down
18 changes: 11 additions & 7 deletions packages/core/src/render3/view_engine_compatibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {assertDefined, assertGreaterThan, assertLessThan} from '../util/assert';

import {NodeInjector, getParentInjectorLocation} from './di';
import {addToViewTree, createEmbeddedViewAndNode, createLContainer, renderEmbeddedTemplate} from './instructions/shared';
import {ACTIVE_INDEX, LContainer, NATIVE, VIEWS} from './interfaces/container';
import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from './interfaces/container';
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
import {RComment, RElement, isProceduralRenderer} from './interfaces/renderer';
import {CONTEXT, LView, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
Expand Down Expand Up @@ -201,14 +201,19 @@ export function createContainerRef(
}

clear(): void {
while (this._lContainer[VIEWS].length) {
while (this._lContainer.length - CONTAINER_HEADER_OFFSET > 0) {
this.remove(0);
}
}

get(index: number): viewEngine_ViewRef|null { return this._viewRefs[index] || null; }

get length(): number { return this._lContainer[VIEWS].length; }
get length(): number {
// Note that if there are no views, the container
// length will be smaller than the header offset.
const viewAmount = this._lContainer.length - CONTAINER_HEADER_OFFSET;
return viewAmount > 0 ? viewAmount : 0;
}

createEmbeddedView<C>(templateRef: ViewEngine_TemplateRef<C>, context?: C, index?: number):
viewEngine_EmbeddedViewRef<C> {
Expand Down Expand Up @@ -250,8 +255,7 @@ export function createContainerRef(

insertView(lView, this._lContainer, adjustedIdx);

const beforeNode =
getBeforeNodeForView(adjustedIdx, this._lContainer[VIEWS], this._lContainer[NATIVE]);
const beforeNode = getBeforeNodeForView(adjustedIdx, this._lContainer);
addRemoveViewFromContainer(lView, true, beforeNode);

(viewRef as ViewRef<any>).attachToViewContainerRef(this);
Expand Down Expand Up @@ -287,12 +291,12 @@ export function createContainerRef(

private _adjustIndex(index?: number, shift: number = 0) {
if (index == null) {
return this._lContainer[VIEWS].length + shift;
return this.length + shift;
}
if (ngDevMode) {
assertGreaterThan(index, -1, 'index must be positive');
// +1 because it's legal to insert at the end.
assertLessThan(index, this._lContainer[VIEWS].length + 1 + shift, 'index');
assertLessThan(index, this.length + 1 + shift, 'index');
}
return index;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
{
"name": "CLEAN_PROMISE"
},
{
"name": "CONTAINER_HEADER_OFFSET"
},
{
"name": "CONTEXT"
},
Expand Down Expand Up @@ -143,9 +146,6 @@
{
"name": "UnsubscriptionErrorImpl"
},
{
"name": "VIEWS"
},
{
"name": "ViewEncapsulation"
},
Expand Down

0 comments on commit 04f8af9

Please sign in to comment.