Skip to content

Commit

Permalink
perf(ivy): apply [style]/[class] bindings directly to style/className
Browse files Browse the repository at this point in the history 
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:
- `<div [class]>` and `<div class="..." [class]>`
- `<div [style]>` and `<div style="..." [style]>`

The overall speec increase is by over 5x.
  • Loading branch information
@matsko
matsko committed Oct 23, 2019
1 parent 398ff1e commit e40ea58
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 71 deletions.
54 changes: 22 additions & 32 deletions packages/core/src/render3/instructions/styling.ts
View file
Expand Up @@ -189,8 +189,7 @@ function stylingProp(
const sanitizerToUse = isClassBased ? null : sanitizer;
const renderer = getRenderer(tNode, lView);
updated = applyStylingValueDirectly(
renderer, context, native, lView, bindingIndex, prop, value, isClassBased,
isClassBased ? setClass : setStyle, sanitizerToUse);
renderer, context, native, lView, bindingIndex, prop, value, isClassBased, sanitizerToUse);

if (sanitizerToUse) {
// it's important we remove the current style sanitizer once the
Expand Down Expand Up @@ -243,6 +242,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
Expand All @@ -253,18 +253,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);
if (ngDevMode) {
ngDevMode.styleMap++;
if (updated) {
ngDevMode.styleMapCacheMiss++;
}
}
stylingMap(context, tNode, lView, bindingIndex, styles, false, hasDirectiveInput);
}

/**
Expand Down Expand Up @@ -300,6 +294,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
Expand All @@ -310,18 +305,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);
if (ngDevMode) {
ngDevMode.classMap++;
if (updated) {
ngDevMode.classMapCacheMiss++;
}
}
stylingMap(context, tNode, lView, bindingIndex, classes, true, hasDirectiveInput);
}

/**
Expand All @@ -331,13 +320,10 @@ export function classMapInternal(
* `[class]` bindings in Angular.
*/
function stylingMap(
elementIndex: number, context: TStylingContext, bindingIndex: number,
value: {[key: string]: any} | string | null, isClassBased: boolean): boolean {
let updated = false;

const lView = getLView();
context: TStylingContext, tNode: TNode, lView: LView, bindingIndex: number,
value: {[key: string]: any} | string | null, isClassBased: boolean,
hasDirectiveInput: boolean): void {
const directiveIndex = getActiveDirectiveId();
const tNode = getTNode(elementIndex, lView);
const native = getNativeByTNode(tNode, lView) as RElement;
const oldValue = getValue(lView, bindingIndex);
const hostBindingsMode = isHostStyling();
Expand All @@ -359,25 +345,24 @@ 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);
applyStylingMapDirectly(
renderer, context, native, lView, bindingIndex, value, isClassBased, 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
// instructions for the next element.
setElementExitFn(stylingApply);
}
} else {
updated = valueHasChanged;
const stylingMapArr =
value === NO_CHANGE ? NO_CHANGE : normalizeIntoStylingMap(oldValue, value, !isClassBased);

activateStylingMapFeature();

// Context Resolution (or first update) Case: save the map value
Expand All @@ -396,7 +381,12 @@ function stylingMap(
setElementExitFn(stylingApply);
}

return updated;
if (ngDevMode) {
isClassBased ? ngDevMode.classMap : ngDevMode.styleMap++;
if (valueHasChanged) {
isClassBased ? ngDevMode.classMapCacheMiss : ngDevMode.styleMapCacheMiss++;
}
}
}

/**
Expand Down
124 changes: 92 additions & 32 deletions packages/core/src/render3/styling/bindings.ts
View file
Expand Up @@ -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';

Expand Down Expand Up @@ -655,44 +655,70 @@ 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;

for (let i = StylingMapArrayIndex.ValuesStartPosition; i < map.length;
i += StylingMapArrayIndex.TupleSize) {
const prop = getMapProp(map, i);
const value = getMapValue(map, i);
bindingIndex: number, value: {[key: string]: any} | string | null, isClassBased: boolean,
sanitizer?: StyleSanitizeFn | null, forceUpdate?: boolean,
bindingValueContainsInitial?: boolean): void {
const config = getConfig(context);
const writeToAttrDirectly = !(config & TStylingConfig.HasPropBindings);
const oldValue = getValue(data, bindingIndex);
if (!writeToAttrDirectly && value !== NO_CHANGE) {
value = normalizeIntoStylingMap(oldValue, value, !isClassBased);
}

// case 1: apply the map value (if it exists)
let applied =
applyStylingValue(renderer, element, prop, value, applyFn, bindingIndex, sanitizer);
if (forceUpdate || hasValueChanged(oldValue, value)) {
const hasInitial = config & TStylingConfig.HasInitialStyling;
setValue(data, bindingIndex, value);

// case 2: apply the initial value (if it exists)
if (!applied && initialStyles) {
applied = findAndApplyMapValue(
renderer, element, applyFn, initialStyles, prop, bindingIndex, sanitizer);
if (writeToAttrDirectly) {
const initialValue =
hasInitial && !bindingValueContainsInitial ? getInitialStylingValue(context) : null;
if (isClassBased) {
let valueToApply = typeof value === 'string' ? value : objectToClassName(value);
if (initialValue !== null) {
valueToApply = initialValue + ' ' + valueToApply;
}
setClassName(renderer, element, valueToApply);
} else {
let valueToApply = forceStylesAsString(value as{[key: string]: any});
if (initialValue !== null) {
valueToApply = initialValue + ';' + valueToApply;
}
setStyleAttr(renderer, element, valueToApply);
}
} else {
const applyFn = isClassBased ? setClass : setStyle;
const map = value as StylingMapArray;
const initialStyles = hasInitial ? getStylingMapArray(context) : null;

for (let i = StylingMapArrayIndex.ValuesStartPosition; i < map.length;
i += StylingMapArrayIndex.TupleSize) {
const prop = getMapProp(map, i);
const value = getMapValue(map, i);

// case 1: apply the map value (if it exists)
let applied =
applyStylingValue(renderer, element, prop, value, applyFn, bindingIndex, sanitizer);

// case 2: apply the initial value (if it exists)
if (!applied && initialStyles) {
applied = findAndApplyMapValue(
renderer, element, applyFn, initialStyles, prop, bindingIndex, sanitizer);
}

// default case: apply `null` to remove the value
if (!applied) {
applyFn(renderer, element, prop, null, bindingIndex);
// default case: apply `null` to remove the value
if (!applied) {
applyFn(renderer, element, prop, null, bindingIndex);
}
}
}

const state = getStylingState(element, TEMPLATE_DIRECTIVE_INDEX);
if (isClassBased) {
state.lastDirectClassMap = map;
} else {
state.lastDirectStyleMap = map;
const state = getStylingState(element, TEMPLATE_DIRECTIVE_INDEX);
if (isClassBased) {
state.lastDirectClassMap = map;
} else {
state.lastDirectStyleMap = map;
}
}

return true;
}
return false;
}

/**
Expand Down Expand Up @@ -727,11 +753,12 @@ export function applyStylingMapDirectly(
*/
export function applyStylingValueDirectly(
renderer: any, context: TStylingContext, element: RElement, data: LStylingData,
bindingIndex: number, prop: string, value: any, isClassBased: boolean, applyFn: ApplyStylingFn,
bindingIndex: number, prop: string, value: any, isClassBased: boolean,
sanitizer?: StyleSanitizeFn | null): boolean {
let applied = false;
if (hasValueChanged(data[bindingIndex], value)) {
setValue(data, bindingIndex, value);
const applyFn = isClassBased ? setClass : setStyle;

// case 1: apply the provided value (if it exists)
applied = applyStylingValue(renderer, element, prop, value, applyFn, bindingIndex, sanitizer);
Expand Down Expand Up @@ -888,6 +915,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.
*
Expand All @@ -914,3 +961,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;
}

0 comments on commit e40ea58

Please sign in to comment.