perf: avoid redundant axisConfigType calculation (#6159)

This may reduce ~5% of compile time
vega · Mar 26, 2020 · 061d300 · 061d300
1 parent aeb57a7
commit 061d300
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 62 deletions.
diff --git a/src/compile/axis/config.ts b/src/compile/axis/config.ts
@@ -1,16 +1,11 @@
 import {Axis} from '../../axis';
-import {PositionScaleChannel} from '../../channel';
 import {Config} from '../../config';
-import {isQuantitative, ScaleType} from '../../scale';
-import {titlecase} from '../../util';
 import {getStyleConfig} from '../common';
 
 export function getAxisConfig(
   property: keyof Axis,
   config: Config,
-  channel: PositionScaleChannel,
-  orient: string,
-  scaleType: ScaleType,
+  axisConfigTypes: string[],
   style: string | string[]
 ) {
   let styleConfig = getStyleConfig(property, style, config.style);
@@ -22,31 +17,9 @@ export function getAxisConfig(
     };
   }
 
-  const typeBasedConfigs = [
-    ...(scaleType === 'band' ? ['axisBand', 'axisDiscrete'] : []),
-    ...(scaleType === 'point' ? ['axisPoint', 'axisDiscrete'] : []),
-    ...(isQuantitative(scaleType) ? ['axisQuantitative'] : []),
-    ...(scaleType === 'time' || scaleType === 'utc' ? ['axisTemporal'] : [])
-  ];
-
-  const channelBasedConfig = channel === 'x' ? 'axisX' : 'axisY';
-
-  // configTypes to loop, starting from higher precedence
-  const axisConfigs = [
-    ...typeBasedConfigs.map(c => channelBasedConfig + c.substr(4)),
-
-    ...typeBasedConfigs,
-    // X/Y
-    channelBasedConfig,
-
-    // axisTop, axisBottom, ...
-    ...(orient ? ['axis' + titlecase(orient)] : []),
-    'axis'
-  ];
-
   // apply properties in config Types first
 
-  for (const configType of axisConfigs) {
+  for (const configType of axisConfigTypes) {
     if (config[configType]?.[property] !== undefined) {
       return {
         configFrom: configType,
@@ -56,7 +29,7 @@ export function getAxisConfig(
   }
 
   // then apply style in config types
-  for (const configType of axisConfigs) {
+  for (const configType of axisConfigTypes) {
     if (config[configType]?.style) {
       styleConfig = getStyleConfig(property, config[configType]?.style, config.style);
       if (styleConfig !== undefined) {

diff --git a/src/compile/axis/parse.ts b/src/compile/axis/parse.ts
@@ -1,4 +1,4 @@
-import {AxisEncode as VgAxisEncode, AxisOrient, SignalRef, Text} from 'vega';
+import {AxisEncode as VgAxisEncode, AxisOrient, ScaleType, SignalRef, Text} from 'vega';
 import {Axis, AXIS_PARTS, isAxisProperty, isConditionalAxisValue} from '../../axis';
 import {isBinned} from '../../bin';
 import {PositionScaleChannel, POSITION_SCALE_CHANNELS, X, Y} from '../../channel';
@@ -12,7 +12,8 @@ import {
   PositionFieldDef,
   toFieldDefBase
 } from '../../channeldef';
-import {contains, getFirstDefined, keys, normalizeAngle} from '../../util';
+import {isQuantitative} from '../../scale';
+import {contains, getFirstDefined, keys, normalizeAngle, titlecase} from '../../util';
 import {isSignalRef} from '../../vega.schema';
 import {mergeTitle, mergeTitleComponent, mergeTitleFieldDefs} from '../common';
 import {numberFormat} from '../format';
@@ -243,6 +244,30 @@ const VEGA_AXIS_CONFIG = {
   axisBand: 1
 };
 
+function getAxisConfigTypes(channel: PositionScaleChannel, scaleType: ScaleType, orient: string) {
+  const typeBasedConfigs = [
+    ...(scaleType === 'band' ? ['axisBand', 'axisDiscrete'] : []),
+    ...(scaleType === 'point' ? ['axisPoint', 'axisDiscrete'] : []),
+    ...(isQuantitative(scaleType) ? ['axisQuantitative'] : []),
+    ...(scaleType === 'time' || scaleType === 'utc' ? ['axisTemporal'] : [])
+  ];
+
+  const channelBasedConfig = channel === 'x' ? 'axisX' : 'axisY';
+
+  // configTypes to loop, starting from higher precedence
+  return [
+    ...typeBasedConfigs.map(c => channelBasedConfig + c.substr(4)),
+
+    ...typeBasedConfigs,
+    // X/Y
+    channelBasedConfig,
+
+    // axisTop, axisBottom, ...
+    ...(orient ? ['axis' + titlecase(orient)] : []),
+    'axis'
+  ];
+}
+
 function parseAxis(channel: PositionScaleChannel, model: UnitModel): AxisComponent {
   const axis = model.axis(channel);
 
@@ -252,18 +277,17 @@ function parseAxis(channel: PositionScaleChannel, model: UnitModel): AxisCompone
     | PositionFieldDef<string>
     | PositionDatumDef<string>;
 
+  const axisConfigTypes = getAxisConfigTypes(
+    channel,
+    model.getScaleComponent(channel).get('type'),
+    getFirstDefined(axis?.orient, properties.orient(channel))
+  );
+
   // 1.2. Add properties
   for (const property of AXIS_COMPONENT_PROPERTIES) {
-    const value = getProperty(fieldOrDatumDef, property, axis, channel, model);
+    const value = getProperty(fieldOrDatumDef, property, axis, channel, model, axisConfigTypes);
     const {configValue = undefined, configFrom = undefined} = isAxisProperty(property)
-      ? getAxisConfig(
-          property,
-          model.config,
-          channel,
-          axisComponent.get('orient'),
-          model.getScaleComponent(channel).get('type'),
-          axis?.style
-        )
+      ? getAxisConfig(property, model.config, axisConfigTypes, axis?.style)
       : {};
 
     const explicit = isExplicit(value, property, axis, model, channel);
@@ -318,7 +342,8 @@ function getProperty<K extends keyof AxisComponentProps>(
   property: K,
   specifiedAxis: Axis,
   channel: PositionScaleChannel,
-  model: UnitModel
+  model: UnitModel,
+  axisConfigTypes: string[]
 ): AxisComponentProps[K] {
   if (property === 'disable') {
     return specifiedAxis !== undefined && (!specifiedAxis as AxisComponentProps[K]);
@@ -360,19 +385,19 @@ function getProperty<K extends keyof AxisComponentProps>(
     }
     case 'labelAlign': {
       const orient = getFirstDefined(specifiedAxis.orient, properties.orient(channel));
-      const labelAngle = properties.labelAngle(model, specifiedAxis, channel, fieldOrDatumDef);
+      const labelAngle = properties.labelAngle(model, specifiedAxis, channel, fieldOrDatumDef, axisConfigTypes);
       return getFirstDefined(
         specifiedAxis.labelAlign,
         properties.defaultLabelAlign(labelAngle, orient)
       ) as AxisComponentProps[K];
     }
     case 'labelAngle': {
-      const labelAngle = properties.labelAngle(model, specifiedAxis, channel, fieldOrDatumDef);
+      const labelAngle = properties.labelAngle(model, specifiedAxis, channel, fieldOrDatumDef, axisConfigTypes);
       return labelAngle as AxisComponentProps[K];
     }
     case 'labelBaseline': {
       const orient = getFirstDefined(specifiedAxis.orient, properties.orient(channel));
-      const labelAngle = properties.labelAngle(model, specifiedAxis, channel, fieldOrDatumDef);
+      const labelAngle = properties.labelAngle(model, specifiedAxis, channel, fieldOrDatumDef, axisConfigTypes);
       return getFirstDefined(
         specifiedAxis.labelBaseline,
         properties.defaultLabelBaseline(labelAngle, orient)

diff --git a/src/compile/axis/properties.ts b/src/compile/axis/properties.ts
@@ -4,7 +4,6 @@ import {Axis} from '../../axis';
 import {isBinning} from '../../bin';
 import {PositionScaleChannel, X, Y} from '../../channel';
 import {DatumDef, isDiscrete, isFieldDef, TypedFieldDef, valueArray} from '../../channeldef';
-import * as log from '../../log';
 import {Mark} from '../../mark';
 import {hasDiscreteDomain, ScaleType} from '../../scale';
 import {normalizeTimeUnit} from '../../timeunit';
@@ -36,21 +35,15 @@ export function labelAngle(
   model: UnitModel,
   specifiedAxis: Axis,
   channel: PositionScaleChannel,
-  fieldOrDatumDef: TypedFieldDef<string> | DatumDef
+  fieldOrDatumDef: TypedFieldDef<string> | DatumDef,
+  axisConfigTypes: string[]
 ) {
   // try axis value
   if (specifiedAxis?.labelAngle !== undefined) {
     return normalizeAngle(specifiedAxis?.labelAngle);
   } else {
     // try axis config value
-    const {configValue: angle} = getAxisConfig(
-      'labelAngle',
-      model.config,
-      channel,
-      orient(channel),
-      model.getScaleComponent(channel).get('type'),
-      specifiedAxis?.style
-    );
+    const {configValue: angle} = getAxisConfig('labelAngle', model.config, axisConfigTypes, specifiedAxis?.style);
     if (angle !== undefined) {
       return normalizeAngle(angle);
     } else {
@@ -137,8 +130,6 @@ export function orient(channel: PositionScaleChannel) {
     case Y:
       return 'left';
   }
-  /* istanbul ignore next: This should never happen. */
-  throw new Error(log.message.INVALID_CHANNEL_FOR_AXIS);
 }
 
 export function defaultTickCount({

diff --git a/test/compile/axis/properties.test.ts b/test/compile/axis/properties.test.ts
@@ -193,23 +193,27 @@ describe('compile/axis', () => {
     });
 
     it('should return the correct labelAngle from the axis definition', () => {
-      expect(240).toEqual(labelAngle(axisModel, axisModel.axis('y'), 'y', axisModel.typedFieldDef('y')));
+      expect(240).toEqual(labelAngle(axisModel, axisModel.axis('y'), 'y', axisModel.typedFieldDef('y'), []));
     });
 
     it('should return the correct labelAngle from the axis config definition', () => {
-      expect(140).toEqual(labelAngle(configModel, configModel.axis('y'), 'y', configModel.typedFieldDef('y')));
+      expect(140).toEqual(
+        labelAngle(configModel, configModel.axis('y'), 'y', configModel.typedFieldDef('y'), ['axis'])
+      );
     });
 
     it('should return the correct default labelAngle when not specified', () => {
-      expect(270).toEqual(labelAngle(defaultModel, defaultModel.axis('x'), 'x', defaultModel.typedFieldDef('x')));
+      expect(270).toEqual(labelAngle(defaultModel, defaultModel.axis('x'), 'x', defaultModel.typedFieldDef('x'), []));
     });
 
     it('should return the labelAngle declared in the axis when both the axis and axis config have labelAngle', () => {
-      expect(240).toEqual(labelAngle(bothModel, bothModel.axis('y'), 'y', bothModel.typedFieldDef('y')));
+      expect(240).toEqual(labelAngle(bothModel, bothModel.axis('y'), 'y', bothModel.typedFieldDef('y'), []));
     });
 
     it('should return undefined when there is no default and no specified labelAngle', () => {
-      expect(undefined).toEqual(labelAngle(neitherModel, neitherModel.axis('y'), 'y', neitherModel.typedFieldDef('y')));
+      expect(undefined).toEqual(
+        labelAngle(neitherModel, neitherModel.axis('y'), 'y', neitherModel.typedFieldDef('y'), [])
+      );
     });
   });