From bd65f5878474bc9b7885c63fbf767d5aee14fe03 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Sat, 23 Feb 2019 09:45:55 -0800 Subject: [PATCH] refactor(ivy): moved wrapped reference to 0 position in array (#28947) `LView`, `LContainer`, `StylingContext` are all arrays which wrap either an `HTMLElement`, `LView`, `LContainer`, `StylingContext`. It is often necessary to retrieve the correct type of element from the location which means that we often have to wrap the arrays. Logically it makes more sense if the thing which we are wrapping is at `0` location. Also it may be more performant since data is more local which may result in more L2 cache hits in CPU. PR Close #28947 --- packages/core/src/render3/instructions.ts | 4 +-- .../core/src/render3/interfaces/container.ts | 26 +++++++------- .../core/src/render3/interfaces/styling.ts | 30 ++++++++-------- packages/core/src/render3/interfaces/view.ts | 26 +++++++------- packages/core/src/render3/styling/util.ts | 2 +- .../styling/class_and_style_bindings_spec.ts | 34 +++++++++---------- 6 files changed, 61 insertions(+), 61 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index eed6a49ee8..8bec067312 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -177,6 +177,7 @@ export function createLView( rendererFactory?: RendererFactory3 | null, renderer?: Renderer3 | null, sanitizer?: Sanitizer | null, injector?: Injector | null): LView { const lView = tView.blueprint.slice() as LView; + lView[HOST] = host; lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.FirstLViewPass; lView[PARENT] = lView[DECLARATION_VIEW] = parentLView; lView[CONTEXT] = context; @@ -186,7 +187,6 @@ export function createLView( ngDevMode && assertDefined(lView[RENDERER], 'Renderer is required'); lView[SANITIZER] = sanitizer || parentLView && parentLView[SANITIZER] || null !; lView[INJECTOR as any] = injector || parentLView && parentLView[INJECTOR] || null; - lView[HOST] = host; lView[T_HOST] = tHostNode; ngDevMode && attachLViewDebug(lView); return lView; @@ -2216,12 +2216,12 @@ export function createLContainer( ngDevMode && assertDomNode(native); ngDevMode && assertLView(currentView); const lContainer: LContainer = [ + hostNative, // host native isForViewContainerRef ? -1 : 0, // active index [], // views currentView, // parent null, // next null, // queries - hostNative, // host native native, // native ]; ngDevMode && attachLContainerDebug(lContainer); diff --git a/packages/core/src/render3/interfaces/container.ts b/packages/core/src/render3/interfaces/container.ts index 7584b93412..5df2b3da48 100644 --- a/packages/core/src/render3/interfaces/container.ts +++ b/packages/core/src/render3/interfaces/container.ts @@ -17,8 +17,8 @@ import {HOST, LView, NEXT, PARENT, QUERIES} from './view'; * without having to remember the specific indices. * Uglify will inline these when minifying so there shouldn't be a cost. */ -export const ACTIVE_INDEX = 0; -export const VIEWS = 1; +export const ACTIVE_INDEX = 1; +export const VIEWS = 2; // PARENT, NEXT, QUERIES, and HOST are indices 2, 3, 4, and 5. // As we already have these constants in LView, we don't need to re-create them. export const NATIVE = 6; @@ -40,6 +40,17 @@ export const LCONTAINER_LENGTH = 7; * of type. */ export interface LContainer extends Array { + /** + * The host element of this LContainer. + * + * The host could be an LView if this container is on a component node. + * In that case, the component LView is its HOST. + * + * It could also be a styling context if this is a node with a style/class + * binding. + */ + readonly[HOST]: RElement|RComment|StylingContext|LView; + /** * The next active index in the views array to read or write to. This helps us * keep track of where we are in the views array. @@ -76,17 +87,6 @@ export interface LContainer extends Array { */ [QUERIES]: LQueries|null; - /** - * The host element of this LContainer. - * - * The host could be an LView if this container is on a component node. - * In that case, the component LView is its HOST. - * - * It could also be a styling context if this is a node with a style/class - * binding. - */ - readonly[HOST]: RElement|RComment|StylingContext|LView; - /** The comment element that serves as an anchor for this LContainer. */ readonly[NATIVE]: RComment; } diff --git a/packages/core/src/render3/interfaces/styling.ts b/packages/core/src/render3/interfaces/styling.ts index c8d90677da..453dff8d3f 100644 --- a/packages/core/src/render3/interfaces/styling.ts +++ b/packages/core/src/render3/interfaces/styling.ts @@ -260,6 +260,11 @@ import {PlayerContext} from './player'; */ export interface StylingContext extends Array<{[key: string]: any}|number|string|boolean|RElement|StyleSanitizeFn|PlayerContext|null> { + /** + * Location of element that is used as a target for this context. + */ + [StylingIndex.ElementPosition]: RElement|null; + /** * A numeric value representing the configuration status (whether the context is dirty or not) * mixed together (using bit shifting) with a index value which tells the starting index value @@ -289,11 +294,6 @@ export interface StylingContext extends */ [StylingIndex.SinglePropOffsetPositions]: SinglePropOffsetValues; - /** - * Location of element that is used as a target for this context. - */ - [StylingIndex.ElementPosition]: RElement|null; - /** * The last class value that was interpreted by elementStylingMap. This is cached * So that the algorithm can exit early incase the value has not changed. @@ -614,18 +614,18 @@ export const enum StylingFlags { /** Used as numeric pointer values to determine what cells to update in the `StylingContext` */ export const enum StylingIndex { - // Index of location where the start of single properties are stored. (`updateStyleProp`) - MasterFlagPosition = 0, - // Position of where the registered directives exist for this styling context - DirectiveRegistryPosition = 1, - // Position of where the initial styles are stored in the styling context - InitialStyleValuesPosition = 2, - InitialClassValuesPosition = 3, - // Index of location where the class index offset value is located - SinglePropOffsetPositions = 4, // Position of where the initial styles are stored in the styling context // This index must align with HOST, see interfaces/view.ts - ElementPosition = 5, + ElementPosition = 0, + // Index of location where the start of single properties are stored. (`updateStyleProp`) + MasterFlagPosition = 1, + // Position of where the registered directives exist for this styling context + DirectiveRegistryPosition = 2, + // Position of where the initial styles are stored in the styling context + InitialStyleValuesPosition = 3, + InitialClassValuesPosition = 4, + // Index of location where the class index offset value is located + SinglePropOffsetPositions = 5, // Position of where the last string-based CSS class value was stored (or a cached version of the // initial styles when a [class] directive is present) CachedMultiClasses = 6, diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index f83e61b9cb..9b6bfc388c 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -27,12 +27,12 @@ import {StylingContext} from './styling'; // Below are constants for LView indices to help us look up LView members // without having to remember the specific indices. // Uglify will inline these when minifying so there shouldn't be a cost. -export const TVIEW = 0; -export const FLAGS = 1; -export const PARENT = 2; -export const NEXT = 3; -export const QUERIES = 4; -export const HOST = 5; +export const HOST = 0; +export const TVIEW = 1; +export const FLAGS = 2; +export const PARENT = 3; +export const NEXT = 4; +export const QUERIES = 5; export const T_HOST = 6; export const BINDING_INDEX = 7; export const CLEANUP = 8; @@ -68,6 +68,13 @@ export interface OpaqueViewState { * don't have to edit the data array based on which views are present. */ export interface LView extends Array { + /** + * The host node for this LView instance, if this is a component view. + * + * If this is an embedded view, HOST will be null. + */ + [HOST]: RElement|StylingContext|null; + /** * The static data for this view. We need a reference to this so we can easily walk up the * node tree in DI and get the TView.data array associated with a node (where the @@ -103,13 +110,6 @@ export interface LView extends Array { /** Queries active for this view - nodes from a view are reported to those queries. */ [QUERIES]: LQueries|null; - /** - * The host node for this LView instance, if this is a component view. - * - * If this is an embedded view, HOST will be null. - */ - [HOST]: RElement|StylingContext|null; - /** * Pointer to the `TViewNode` or `TElementNode` which represents the root of the view. * diff --git a/packages/core/src/render3/styling/util.ts b/packages/core/src/render3/styling/util.ts index 3984d699e8..af6f215cf0 100644 --- a/packages/core/src/render3/styling/util.ts +++ b/packages/core/src/render3/styling/util.ts @@ -27,12 +27,12 @@ export function createEmptyStylingContext( initialStyles?: InitialStylingValues | null, initialClasses?: InitialStylingValues | null): StylingContext { const context: StylingContext = [ + element || null, // Element 0, // MasterFlags [] as any, // DirectiveRefs (this gets filled below) initialStyles || [null, null], // InitialStyles initialClasses || [null, null], // InitialClasses [0, 0], // SinglePropOffsets - element || null, // Element [0], // CachedMultiClassValue [0], // CachedMultiStyleValue null, // PlayerContext diff --git a/packages/core/test/render3/styling/class_and_style_bindings_spec.ts b/packages/core/test/render3/styling/class_and_style_bindings_spec.ts index d91a41aa2f..2ea84900b3 100644 --- a/packages/core/test/render3/styling/class_and_style_bindings_spec.ts +++ b/packages/core/test/render3/styling/class_and_style_bindings_spec.ts @@ -200,12 +200,12 @@ describe('style and class based bindings', () => { it('should initialize empty template', () => { const template = initContext(); assertContext(template, [ + element, masterConfig(9), [null, 2, false, null], [null, null], [null, null], [0, 0, 0, 0], - element, [0, 0, 9, null, 0], [0, 0, 9, null, 0], null, @@ -215,12 +215,12 @@ describe('style and class based bindings', () => { it('should initialize static styles and classes', () => { const template = initContext(['color', 'red', 'width', '10px'], null, ['foo', 'bar']); assertContext(template, [ + element, masterConfig(9), [null, 2, false, null], [null, null, 'color', 'red', 'width', '10px'], [null, null, 'foo', true, 'bar', true], [0, 0, 0, 0], - element, [0, 0, 9, null, 0], [0, 0, 9, null, 0], null, @@ -437,12 +437,12 @@ describe('style and class based bindings', () => { updateContextWithBindings(ctx, null, ['foo'], ['width']); assertContext(ctx, [ + null, masterConfig(17, false, false), // [null, 2, false, null], [null, null, 'width', null], [null, null, 'foo', false], [1, 1, 1, 1, 9, 13], - null, [1, 0, 21, null, 1], [1, 0, 17, null, 1], null, @@ -475,12 +475,12 @@ describe('style and class based bindings', () => { updateContextWithBindings(ctx, 'SOME DIRECTIVE', ['bar'], ['width', 'height']); assertContext(ctx, [ + null, masterConfig(25, false, false), // [null, 2, false, null, 'SOME DIRECTIVE', 6, false, null], [null, null, 'width', null, 'height', null], [null, null, 'foo', false, 'bar', false], [2, 2, 1, 1, 9, 17, 2, 1, 9, 13, 21], - null, [2, 0, 33, null, 1, 0, 37, null, 1], [2, 0, 25, null, 1, 0, 29, null, 1], null, @@ -538,6 +538,7 @@ describe('style and class based bindings', () => { ctx, 'SOME DIRECTIVE 2', ['baz', 'bar', 'foo'], ['opacity', 'width', 'height']); assertContext(ctx, [ + null, masterConfig(33, false, false), // [ null, 2, false, null, 'SOME DIRECTIVE', 6, false, null, 'SOME DIRECTIVE 2', 11, @@ -546,7 +547,6 @@ describe('style and class based bindings', () => { [null, null, 'width', null, 'height', null, 'opacity', null], [null, null, 'foo', false, 'bar', false, 'baz', false], [3, 3, 1, 1, 9, 21, 2, 1, 9, 13, 25, 3, 3, 17, 9, 13, 29, 25, 21], - null, [3, 0, 45, null, 1, 0, 49, null, 1, 0, 53, null, 1], [3, 0, 33, null, 1, 0, 37, null, 1, 0, 41, null, 1], null, @@ -1145,12 +1145,12 @@ describe('style and class based bindings', () => { updateStyleProp(stylingContext, 0, '200px'); assertContext(stylingContext, [ + element, masterConfig(13, true), // [null, 2, true, null], [null, null, 'height', null], [null, null], [1, 0, 1, 0, 9], - element, [0, 0, 21, null, 0], [1, 0, 13, cachedStyleValue, 1], null, @@ -1177,12 +1177,12 @@ describe('style and class based bindings', () => { getStyles(stylingContext); assertContext(stylingContext, [ + element, masterConfig(13, false), // [null, 2, false, null], [null, null, 'height', null], [null, null], [1, 0, 1, 0, 9], - element, [0, 0, 21, null, 0], [1, 0, 13, cachedStyleValue, 1], null, @@ -1846,12 +1846,12 @@ describe('style and class based bindings', () => { it('should initialize with the provided class bindings', () => { const template = initContext(null, null, null, ['one', 'two']); assertContext(template, [ + element, masterConfig(17, false), // [null, 2, false, null], [null, null], [null, null, 'one', false, 'two', false], [0, 2, 0, 2, 9, 13], - element, [2, 0, 17, null, 2], [0, 0, 17, null, 0], null, @@ -1927,12 +1927,12 @@ describe('style and class based bindings', () => { const stylingContext = initContext(['width', '100px'], ['width', 'height'], ['wide'], ['wide', 'tall']); assertContext(stylingContext, [ + element, masterConfig(25, false), // [null, 2, false, null], [null, null, 'width', '100px', 'height', null], [null, null, 'wide', true, 'tall', false], [2, 2, 2, 2, 9, 13, 17, 21], - element, [2, 0, 33, null, 2], [2, 0, 25, null, 2], null, @@ -1991,12 +1991,12 @@ describe('style and class based bindings', () => { let cachedStyleMap: any = {width: '200px', opacity: '0.5'}; updateStylingMap(stylingContext, 'tall round', cachedStyleMap); assertContext(stylingContext, [ + element, masterConfig(25, true), // [null, 2, true, null], [null, null, 'width', '100px', 'height', null], [null, null, 'wide', true, 'tall', false], [2, 2, 2, 2, 9, 13, 17, 21], - element, [2, 0, 37, 'tall round', 2], [2, 0, 25, cachedStyleMap, 2], null, @@ -2073,12 +2073,12 @@ describe('style and class based bindings', () => { updateStyleProp(stylingContext, 0, '300px'); assertContext(stylingContext, [ + element, masterConfig(25, true), // [null, 2, true, null], [null, null, 'width', '100px', 'height', null], [null, null, 'wide', true, 'tall', false], [2, 2, 2, 2, 9, 13, 17, 21], - element, [2, 0, 37, cachedClassMap, 2], [1, 0, 25, cachedStyleMap, 1], null, @@ -2169,12 +2169,12 @@ describe('style and class based bindings', () => { getStylesAndClasses(stylingContext); assertContext(stylingContext, [ + element, // masterConfig(9, false), // [null, 2, false, null], // [null, null], // [null, null], // [0, 0, 0, 0], // - element, // [1, 0, 13, classesMap, 1], // [1, 0, 9, stylesMap, 1], // null, // @@ -2195,12 +2195,12 @@ describe('style and class based bindings', () => { getStylesAndClasses(stylingContext); assertContext(stylingContext, [ + element, // masterConfig(9, false), // [null, 2, false, null], // [null, null], // [null, null], // [0, 0, 0, 0], // - element, // [1, 0, 13, classesMap, 1], // [1, 0, 9, stylesMap, 1], // null, // @@ -2224,12 +2224,12 @@ describe('style and class based bindings', () => { expect(getClasses(stylingContext)).toEqual({apple: true, orange: true, banana: true}); assertContext(stylingContext, [ + element, masterConfig(9, false), // [null, 2, false, null], [null, null], [null, null], [0, 0, 0, 0], - element, [3, 0, 9, 'apple orange banana', 3], [0, 0, 9, null, 0], null, @@ -2590,12 +2590,12 @@ describe('style and class based bindings', () => { }; assertContext(context, [ + element, // masterConfig(17, false), // [null, 2, false, null], // [null, null, 'color', null], // [null, null, 'foo', false], // [1, 1, 1, 1, 9, 13], // - element, // [1, 0, 21, null, 1], // [1, 0, 17, null, 1], // null, // @@ -2653,12 +2653,12 @@ describe('style and class based bindings', () => { ] as PlayerContext); assertContext(context, [ + element, // masterConfig(17, false), // [null, 2, false, null], // [null, null, 'color', null], // [null, null, 'foo', false], // [1, 1, 1, 1, 9, 13], // - element, // [1, 0, 25, classMapWithPlayerFactory, 1], // [1, 0, 17, styleMapWithPlayerFactory, 1], // playerContext, @@ -2714,12 +2714,12 @@ describe('style and class based bindings', () => { ] as PlayerContext); assertContext(context, [ + element, // masterConfig(17, false), // [null, 2, false, null], // [null, null, 'color', null], // [null, null, 'foo', false], // [1, 1, 1, 1, 9, 13], // - element, // [1, 0, 25, cachedClassMap, 1], // [1, 0, 17, cachedStyleMap, 1], // playerContext,