perf(ivy): apply [style]/[class] bindings directly to style/className (…

…angular#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 angular#33336
justsml · Oct 25, 2019 · dcdb433 · dcdb433
1 parent ee4fc12
commit dcdb433
Show file tree

Hide file tree

Showing 13 changed files with 360 additions and 103 deletions.
diff --git a/packages/common/src/directives/ng_class.ts b/packages/common/src/directives/ng_class.ts
@@ -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());

diff --git a/packages/common/src/directives/ng_style.ts b/packages/common/src/directives/ng_style.ts
@@ -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());

diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts
@@ -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");
@@ -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");
@@ -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");
@@ -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) {
@@ -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) {
@@ -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));
             }
           }
           `;
@@ -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, "");
             }
@@ -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) {
@@ -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);
@@ -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);
@@ -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);
@@ -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) {

diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts
@@ -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

diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts
@@ -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);
@@ -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) {
@@ -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
@@ -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);
 }
 
 /**
@@ -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);
 }
 
 /**
@@ -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();
@@ -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
@@ -396,7 +381,12 @@ function stylingMap(
     setElementExitFn(stylingApply);
   }
 
-  return updated;
+  if (ngDevMode) {
+    isClassBased ? ngDevMode.classMap : ngDevMode.styleMap++;
+    if (valueHasChanged) {
+      isClassBased ? ngDevMode.classMapCacheMiss : ngDevMode.styleMapCacheMiss++;
+    }
+  }
 }
 
 /**
@@ -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;
@@ -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;
+}