Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
perf(ivy): apply [style]/[class] bindings directly to style/className (
…#33336)

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.

PR Close #33336
  • Loading branch information
matsko authored and AndrewKushnir committed Oct 25, 2019
1 parent ee4fc12 commit dcdb433
Show file tree
Hide file tree
Showing 13 changed files with 360 additions and 103 deletions.
2 changes: 1 addition & 1 deletion packages/common/src/directives/ng_class.ts
Expand Up @@ -34,7 +34,7 @@ export const ngClassDirectiveDef__POST_R3__ = ɵɵdefineDirective({
selectors: null as any,
hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) {
if (rf & ɵRenderFlags.Create) {
ɵɵallocHostVars(1);
ɵɵallocHostVars(2);
}
if (rf & ɵRenderFlags.Update) {
ɵɵclassMap(ctx.getValue());
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/directives/ng_style.ts
Expand Up @@ -35,7 +35,7 @@ export const ngStyleDirectiveDef__POST_R3__ = ɵɵdefineDirective({
selectors: null as any,
hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) {
if (rf & ɵRenderFlags.Create) {
ɵɵallocHostVars(1);
ɵɵallocHostVars(2);
}
if (rf & ɵRenderFlags.Update) {
ɵɵstyleMap(ctx.getValue());
Expand Down
Expand Up @@ -436,7 +436,7 @@ describe('compiler compliance: styling', () => {
const template = `
decls: 1,
vars: 2,
vars: 3,
template: function MyComponentWithInterpolation_Template(rf, $ctx$) {
if (rf & 1) {
$r3$.ɵɵelement(0, "div");
Expand All @@ -447,7 +447,7 @@ describe('compiler compliance: styling', () => {
}
decls: 1,
vars: 3,
vars: 4,
template: function MyComponentWithMuchosInterpolation_Template(rf, $ctx$) {
if (rf & 1) {
$r3$.ɵɵelement(0, "div");
Expand All @@ -458,7 +458,7 @@ describe('compiler compliance: styling', () => {
}
decls: 1,
vars: 1,
vars: 2,
template: function MyComponentWithoutInterpolation_Template(rf, $ctx$) {
if (rf & 1) {
$r3$.ɵɵelement(0, "div");
Expand Down Expand Up @@ -506,7 +506,7 @@ describe('compiler compliance: styling', () => {
type: MyComponent,
selectors:[["my-component"]],
decls: 1,
vars: 4,
vars: 5,
consts: [[${AttributeMarker.Styles}, "opacity", "1"]],
template: function MyComponent_Template(rf, $ctx$) {
if (rf & 1) {
Expand Down Expand Up @@ -700,7 +700,7 @@ describe('compiler compliance: styling', () => {
type: MyComponent,
selectors:[["my-component"]],
decls: 1,
vars: 4,
vars: 5,
consts: [[${AttributeMarker.Classes}, "grape"]],
template: function MyComponent_Template(rf, $ctx$) {
if (rf & 1) {
Expand Down Expand Up @@ -863,8 +863,8 @@ describe('compiler compliance: styling', () => {
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 2, $ctx$.myStyleExp));
$r3$.ɵɵclassMap($r3$.ɵɵpipeBind1(2, 4, $ctx$.myClassExp));
$r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 4, $ctx$.myStyleExp));
$r3$.ɵɵclassMap($r3$.ɵɵpipeBind1(2, 6, $ctx$.myClassExp));
}
}
`;
Expand Down Expand Up @@ -916,11 +916,11 @@ describe('compiler compliance: styling', () => {
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 6, $ctx$.myStyleExp, 1000));
$r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 8, $ctx$.myStyleExp, 1000));
$r3$.ɵɵclassMap($e2_styling$);
$r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 9, $ctx$.barExp, 3000));
$r3$.ɵɵstyleProp("baz", $r3$.ɵɵpipeBind2(3, 12, $ctx$.bazExp, 4000));
$r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 15, $ctx$.fooExp, 2000));
$r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 11, $ctx$.barExp, 3000));
$r3$.ɵɵstyleProp("baz", $r3$.ɵɵpipeBind2(3, 14, $ctx$.bazExp, 4000));
$r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 17, $ctx$.fooExp, 2000));
$r3$.ɵɵadvance(5);
$r3$.ɵɵtextInterpolate1(" ", $ctx$.item, "");
}
Expand Down Expand Up @@ -1018,7 +1018,7 @@ describe('compiler compliance: styling', () => {
const template = `
hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
if (rf & 1) {
$r3$.ɵɵallocHostVars(4);
$r3$.ɵɵallocHostVars(6);
$r3$.ɵɵelementHostAttrs($e0_attrs$);
}
if (rf & 2) {
Expand Down Expand Up @@ -1077,7 +1077,7 @@ describe('compiler compliance: styling', () => {
const template = `
hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
if (rf & 1) {
$r3$.ɵɵallocHostVars(6);
$r3$.ɵɵallocHostVars(8);
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
Expand Down Expand Up @@ -1152,7 +1152,7 @@ describe('compiler compliance: styling', () => {
const hostBindings = `
hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
if (rf & 1) {
$r3$.ɵɵallocHostVars(4);
$r3$.ɵɵallocHostVars(6);
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
Expand Down Expand Up @@ -1218,7 +1218,7 @@ describe('compiler compliance: styling', () => {
const template = `
function ClassDirective_HostBindings(rf, ctx, elIndex) {
if (rf & 1) {
$r3$.ɵɵallocHostVars(1);
$r3$.ɵɵallocHostVars(2);
}
if (rf & 2) {
$r3$.ɵɵclassMap(ctx.myClassMap);
Expand Down Expand Up @@ -1506,7 +1506,7 @@ describe('compiler compliance: styling', () => {
hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
if (rf & 1) {
$r3$.ɵɵallocHostVars(4);
$r3$.ɵɵallocHostVars(6);
$r3$.ɵɵelementHostAttrs($_c0$);
}
if (rf & 2) {
Expand Down
5 changes: 4 additions & 1 deletion packages/compiler/src/render3/view/styling_builder.ts
Expand Up @@ -326,7 +326,10 @@ export class StylingBuilder {
valueConverter: ValueConverter, isClassBased: boolean,
stylingInput: BoundStylingEntry): StylingInstruction {
// each styling binding value is stored in the LView
let totalBindingSlotsRequired = 1;
// map-based bindings allocate two slots: one for the
// previous binding value and another for the previous
// className or style attribute value.
let totalBindingSlotsRequired = 2;

// these values must be outside of the update block so that they can
// be evaluated (the AST visit call) during creation time so that any
Expand Down
74 changes: 36 additions & 38 deletions packages/core/src/render3/instructions/styling.ts
Expand Up @@ -96,7 +96,7 @@ export function stylePropInternal(
// in this case we do not need to do anything, but the binding index
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = lView[BINDING_INDEX]++;
const bindingIndex = getAndIncrementBindingIndex(lView, false);

const updated =
stylingProp(elementIndex, bindingIndex, prop, resolveStylePropValue(value, suffix), false);
Expand Down Expand Up @@ -130,7 +130,7 @@ export function ɵɵclassProp(className: string, value: boolean | null): void {
// in this case we do not need to do anything, but the binding index
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = lView[BINDING_INDEX]++;
const bindingIndex = getAndIncrementBindingIndex(lView, false);

const updated = stylingProp(getSelectedIndex(), bindingIndex, className, value, true);
if (ngDevMode) {
Expand Down 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,28 +242,23 @@ 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
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = lView[BINDING_INDEX]++;
const bindingIndex = getAndIncrementBindingIndex(lView, true);

// 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,28 +294,23 @@ 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
// still needs to be incremented because all styling binding values
// are stored inside of the lView.
const bindingIndex = lView[BINDING_INDEX]++;
const bindingIndex = getAndIncrementBindingIndex(lView, true);

// 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 Expand Up @@ -452,8 +442,8 @@ function normalizeStylingDirectiveInputValue(
value = concatString(initialValue, forceClassesAsString(bindingValue));
} else {
value = concatString(
initialValue, forceStylesAsString(bindingValue as{[key: string]: any} | null | undefined),
';');
initialValue,
forceStylesAsString(bindingValue as{[key: string]: any} | null | undefined, true), ';');
}
}
return value;
Expand Down Expand Up @@ -594,3 +584,11 @@ function resolveStylePropValue(
function isHostStyling(): boolean {
return isHostStylingActive(getActiveDirectiveId());
}

function getAndIncrementBindingIndex(lView: LView, isMapBased: boolean): number {
// map-based bindings use two slots because the previously constructed
// className / style value must be compared against.
const index = lView[BINDING_INDEX];
lView[BINDING_INDEX] += isMapBased ? 2 : 1;
return index;
}

0 comments on commit dcdb433

Please sign in to comment.