From 0911796ab2b6e4a2070850c928ea5b3fde69e10c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 22 Oct 2019 15:18:40 -0700 Subject: [PATCH] perf(ivy): apply [style]/[class] bindings directly to style/className This patch ensures that the `[style]` and `[class]` based bindings are directly applied to an element's style and className attributes. This patch optimizes the algorithm so that it... - Doesn't construct an update an instance of `StylingMapArray` for `[style]` and `[class]` bindings - Doesn't apply `[style]` and `[class]` based entries using `classList` and `style` (direct attributes are used instead) - Doesn't split or iterate over all string-based tokens in a string value obtained from a `[class]` binding. This patch speeds up the following cases: - `
` and `
` - `
` and `
` The overall speec increase is by over 5x. --- .../core/src/render3/instructions/styling.ts | 24 +++--- packages/core/src/render3/styling/bindings.ts | 72 ++++++++++++++++-- .../core/src/render3/styling/styling_debug.ts | 74 +++++++++++++++++-- 3 files changed, 145 insertions(+), 25 deletions(-) diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index 168de69f36bd0..14b415ee08b7a 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -243,6 +243,7 @@ export function ɵɵstyleMap(styles: {[styleName: string]: any} | NO_CHANGE | nu const lView = getLView(); const tNode = getTNode(index, lView); const context = getStylesContext(tNode); + const hasDirectiveInput = hasStyleInput(tNode); // if a value is interpolated then it may render a `NO_CHANGE` value. // in this case we do not need to do anything, but the binding index @@ -253,12 +254,12 @@ export function ɵɵstyleMap(styles: {[styleName: string]: any} | NO_CHANGE | nu // inputs are only evaluated from a template binding into a directive, therefore, // there should not be a situation where a directive host bindings function // evaluates the inputs (this should only happen in the template function) - if (!isHostStyling() && hasStyleInput(tNode) && styles !== NO_CHANGE) { + if (!isHostStyling() && hasDirectiveInput && styles !== NO_CHANGE) { updateDirectiveInputValue(context, lView, tNode, bindingIndex, styles, false); styles = NO_CHANGE; } - const updated = stylingMap(index, context, bindingIndex, styles, false); + const updated = stylingMap(index, context, bindingIndex, styles, false, hasDirectiveInput); if (ngDevMode) { ngDevMode.styleMap++; if (updated) { @@ -300,6 +301,7 @@ export function classMapInternal( const lView = getLView(); const tNode = getTNode(elementIndex, lView); const context = getClassesContext(tNode); + const hasDirectiveInput = hasClassInput(tNode); // if a value is interpolated then it may render a `NO_CHANGE` value. // in this case we do not need to do anything, but the binding index @@ -310,12 +312,12 @@ export function classMapInternal( // inputs are only evaluated from a template binding into a directive, therefore, // there should not be a situation where a directive host bindings function // evaluates the inputs (this should only happen in the template function) - if (!isHostStyling() && hasClassInput(tNode) && classes !== NO_CHANGE) { + if (!isHostStyling() && hasDirectiveInput && classes !== NO_CHANGE) { updateDirectiveInputValue(context, lView, tNode, bindingIndex, classes, true); classes = NO_CHANGE; } - const updated = stylingMap(elementIndex, context, bindingIndex, classes, true); + const updated = stylingMap(elementIndex, context, bindingIndex, classes, true, hasDirectiveInput); if (ngDevMode) { ngDevMode.classMap++; if (updated) { @@ -332,9 +334,9 @@ export function classMapInternal( */ function stylingMap( elementIndex: number, context: TStylingContext, bindingIndex: number, - value: {[key: string]: any} | string | null, isClassBased: boolean): boolean { + value: {[key: string]: any} | string | null, isClassBased: boolean, + hasDirectiveInput: boolean): boolean { let updated = false; - const lView = getLView(); const directiveIndex = getActiveDirectiveId(); const tNode = getTNode(elementIndex, lView); @@ -359,17 +361,14 @@ function stylingMap( patchConfig(context, TStylingConfig.HasMapBindings); } - const stylingMapArr = - value === NO_CHANGE ? NO_CHANGE : normalizeIntoStylingMap(oldValue, value, !isClassBased); - // Direct Apply Case: bypass context resolution and apply the // style/class map values directly to the element if (allowDirectStyling(context, hostBindingsMode)) { const sanitizerToUse = isClassBased ? null : sanitizer; const renderer = getRenderer(tNode, lView); updated = applyStylingMapDirectly( - renderer, context, native, lView, bindingIndex, stylingMapArr as StylingMapArray, - isClassBased, isClassBased ? setClass : setStyle, sanitizerToUse, valueHasChanged); + renderer, context, native, lView, bindingIndex, value, isClassBased, + isClassBased ? setClass : setStyle, sanitizerToUse, valueHasChanged, hasDirectiveInput); if (sanitizerToUse) { // it's important we remove the current style sanitizer once the // element exits, otherwise it will be used by the next styling @@ -377,6 +376,9 @@ function stylingMap( setElementExitFn(stylingApply); } } else { + const stylingMapArr = + value === NO_CHANGE ? NO_CHANGE : normalizeIntoStylingMap(oldValue, value, !isClassBased); + updated = valueHasChanged; activateStylingMapFeature(); diff --git a/packages/core/src/render3/styling/bindings.ts b/packages/core/src/render3/styling/bindings.ts index 8d9453e23767f..211e228273e84 100644 --- a/packages/core/src/render3/styling/bindings.ts +++ b/packages/core/src/render3/styling/bindings.ts @@ -10,7 +10,7 @@ import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanit import {ProceduralRenderer3, RElement, Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer'; import {ApplyStylingFn, LStylingData, StylingMapArray, StylingMapArrayIndex, StylingMapsSyncMode, SyncStylingMapsFn, TStylingConfig, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from '../interfaces/styling'; import {NO_CHANGE} from '../tokens'; -import {DEFAULT_BINDING_INDEX, DEFAULT_BINDING_VALUE, DEFAULT_GUARD_MASK_VALUE, MAP_BASED_ENTRY_PROP_NAME, TEMPLATE_DIRECTIVE_INDEX, getBindingValue, getConfig, getDefaultValue, getGuardMask, getInitialStylingValue, getMapProp, getMapValue, getProp, getPropValuesStartPosition, getStylingMapArray, getTotalSources, getValue, getValuesCount, hasConfig, hasValueChanged, isContextLocked, isHostStylingActive, isSanitizationRequired, isStylingValueDefined, lockContext, patchConfig, setDefaultValue, setGuardMask, setMapAsDirty, setValue} from '../util/styling_utils'; +import {DEFAULT_BINDING_INDEX, DEFAULT_BINDING_VALUE, DEFAULT_GUARD_MASK_VALUE, MAP_BASED_ENTRY_PROP_NAME, TEMPLATE_DIRECTIVE_INDEX, forceStylesAsString, getBindingValue, getConfig, getDefaultValue, getGuardMask, getInitialStylingValue, getMapProp, getMapValue, getProp, getPropValuesStartPosition, getStylingMapArray, getTotalSources, getValue, getValuesCount, hasConfig, hasValueChanged, isContextLocked, isHostStylingActive, isSanitizationRequired, isStylingValueDefined, lockContext, normalizeIntoStylingMap, patchConfig, setDefaultValue, setGuardMask, setMapAsDirty, setValue} from '../util/styling_utils'; import {getStylingState, resetStylingState} from './state'; @@ -655,12 +655,37 @@ export function applyStylingViaContext( */ export function applyStylingMapDirectly( renderer: any, context: TStylingContext, element: RElement, data: LStylingData, - bindingIndex: number, map: StylingMapArray, isClassBased: boolean, applyFn: ApplyStylingFn, - sanitizer?: StyleSanitizeFn | null, forceUpdate?: boolean): boolean { - if (forceUpdate || hasValueChanged(data[bindingIndex], map)) { - setValue(data, bindingIndex, map); - const initialStyles = - hasConfig(context, TStylingConfig.HasInitialStyling) ? getStylingMapArray(context) : null; + bindingIndex: number, value: {[key: string]: any} | string | null, isClassBased: boolean, + applyFn: ApplyStylingFn, sanitizer?: StyleSanitizeFn | null, forceUpdate?: boolean, + bindingValueContainsInitial?: boolean): boolean { + const config = getConfig(context); + const writeToAttrDirectly = !(config & TStylingConfig.HasPropBindings); + const oldValue = getValue(data, bindingIndex); + if (!writeToAttrDirectly && value !== NO_CHANGE) { + value = normalizeIntoStylingMap(oldValue, value, !isClassBased); + } + + if (forceUpdate || hasValueChanged(oldValue, value)) { + const hasInitial = config & TStylingConfig.HasInitialStyling; + setValue(data, bindingIndex, value); + + if (writeToAttrDirectly) { + const concatToken = isClassBased ? ' ' : ';'; + let valueToApply = hasInitial && !bindingValueContainsInitial ? + getInitialStylingValue(context) + concatToken : + ''; + if (isClassBased) { + valueToApply += typeof value === 'string' ? value : objectToClassName(value); + setClassName(renderer, element, valueToApply); + } else { + valueToApply += forceStylesAsString(value as{[key: string]: any}); + setStyleAttr(renderer, element, valueToApply); + } + return true; + } + + const map = value as StylingMapArray; + const initialStyles = hasInitial ? getStylingMapArray(context) : null; for (let i = StylingMapArrayIndex.ValuesStartPosition; i < map.length; i += StylingMapArrayIndex.TupleSize) { @@ -888,6 +913,26 @@ export const setClass: ApplyStylingFn = } }; +export const setClassName = (renderer: Renderer3 | null, native: RElement, className: string) => { + if (renderer !== null) { + if (isProceduralRenderer(renderer)) { + renderer.setAttribute(native, 'class', className); + } else { + native.className = className; + } + } +}; + +export const setStyleAttr = (renderer: Renderer3 | null, native: RElement, value: string) => { + if (renderer !== null) { + if (isProceduralRenderer(renderer)) { + renderer.setAttribute(native, 'style', value); + } else { + native.setAttribute('style', value); + } + } +}; + /** * Iterates over all provided styling entries and renders them on the element. * @@ -914,3 +959,16 @@ export function renderStylingMap( } } } + +function objectToClassName(obj: {[key: string]: any} | null): string { + let str = ''; + if (obj) { + for (let key in obj) { + const value = obj[key]; + if (value) { + str += (str.length ? ' ' : '') + key; + } + } + } + return str; +} diff --git a/packages/core/src/render3/styling/styling_debug.ts b/packages/core/src/render3/styling/styling_debug.ts index 7b9e108c597e8..981f46d6218f9 100644 --- a/packages/core/src/render3/styling/styling_debug.ts +++ b/packages/core/src/render3/styling/styling_debug.ts @@ -5,12 +5,13 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {createProxy} from '../../debug/proxy'; import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; import {RElement} from '../interfaces/renderer'; import {ApplyStylingFn, LStylingData, TStylingConfig, TStylingContext, TStylingContextIndex} from '../interfaces/styling'; import {getCurrentStyleSanitizer} from '../state'; import {attachDebugObject} from '../util/debug_utils'; -import {MAP_BASED_ENTRY_PROP_NAME, TEMPLATE_DIRECTIVE_INDEX, allowDirectStyling as _allowDirectStyling, getBindingValue, getDefaultValue, getGuardMask, getProp, getPropValuesStartPosition, getValuesCount, hasConfig, isContextLocked, isSanitizationRequired, isStylingContext} from '../util/styling_utils'; +import {MAP_BASED_ENTRY_PROP_NAME, TEMPLATE_DIRECTIVE_INDEX, allowDirectStyling as _allowDirectStyling, getBindingValue, getDefaultValue, getGuardMask, getProp, getPropValuesStartPosition, getValue, getValuesCount, hasConfig, isContextLocked, isSanitizationRequired, isStylingContext, normalizeIntoStylingMap, setValue} from '../util/styling_utils'; import {applyStylingViaContext} from './bindings'; import {activateStylingMapFeature} from './map_based_bindings'; @@ -374,10 +375,52 @@ export class NodeStylingDebug implements DebugNodeStyling { */ get summary(): {[key: string]: DebugNodeStylingEntry} { const entries: {[key: string]: DebugNodeStylingEntry} = {}; - this._mapValues((prop: string, value: any, bindingIndex: number | null) => { + const config = this.config; + const isClassBased = this._isClassBased; + + let data = this._data; + + // the direct pass code doesn't convert [style] or [class] values + // into StylingMapArray instances. For this reason, the values + // need to be converted ahead of time since the styling debug + // relies on context resolution to figure out what styling + // values have been added/removed on the element. + if (config.allowDirectStyling && config.hasMapBindings) { + data = data.concat([]); // make a copy + this._convertMapBindingsToStylingMapArrays(data); + } + + this._mapValues(data, (prop: string, value: any, bindingIndex: number | null) => { entries[prop] = {prop, value, bindingIndex}; }); - return entries; + + // because the styling algorithm runs into two different + // modes: direct and context-resolution, the output of the entries + // object is different because the removed values are not + // saved between updates. For this reason a proxy is created + // so that the behavior is the same when examining values + // that are no longer active on the element. + return createProxy({ + get(target: {}, prop: string): DebugNodeStylingEntry{ + let value: DebugNodeStylingEntry = entries[prop]; if (!value) { + value = { + prop, + value: isClassBased ? false : null, + bindingIndex: null, + }; + } return value; + }, + set(target: {}, prop: string, value: any) { return false; }, + ownKeys() { return Object.keys(entries); }, + getOwnPropertyDescriptor(k: any) { + // we use a special property descriptor here so that enumeration operations + // such as `Object.keys` will work on this proxy. + return { + enumerable: true, + configurable: true, + }; + }, + }); } get config() { return buildConfig(this.context.context); } @@ -387,11 +430,28 @@ export class NodeStylingDebug implements DebugNodeStyling { */ get values(): {[key: string]: any} { const entries: {[key: string]: any} = {}; - this._mapValues((prop: string, value: any) => { entries[prop] = value; }); + this._mapValues(this._data, (prop: string, value: any) => { entries[prop] = value; }); return entries; } - private _mapValues(fn: (prop: string, value: string|null, bindingIndex: number|null) => any) { + private _convertMapBindingsToStylingMapArrays(data: LStylingData) { + const context = this.context.context; + const limit = getPropValuesStartPosition(context); + for (let i = + TStylingContextIndex.ValuesStartPosition + TStylingContextIndex.BindingsStartOffset; + i < limit; i++) { + const bindingIndex = context[i] as number; + const bindingValue = bindingIndex !== 0 ? getValue(data, bindingIndex) : null; + if (bindingValue && !Array.isArray(bindingValue)) { + const stylingMapArray = normalizeIntoStylingMap(null, bindingValue, !this._isClassBased); + setValue(data, bindingIndex, stylingMapArray); + } + } + } + + private _mapValues( + data: LStylingData, + fn: (prop: string, value: string|null, bindingIndex: number|null) => any) { // there is no need to store/track an element instance. The // element is only used when the styling algorithm attempts to // style the value (and we mock out the stylingApplyFn anyway). @@ -409,11 +469,11 @@ export class NodeStylingDebug implements DebugNodeStyling { // run the template bindings applyStylingViaContext( - this.context.context, null, mockElement, this._data, true, mapFn, sanitizer, false); + this.context.context, null, mockElement, data, true, mapFn, sanitizer, false); // and also the host bindings applyStylingViaContext( - this.context.context, null, mockElement, this._data, true, mapFn, sanitizer, true); + this.context.context, null, mockElement, data, true, mapFn, sanitizer, true); } }