Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: use more sets and reduce usage of contains #7745

Merged
merged 3 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 25 additions & 11 deletions src/aggregate.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {AggregateOp} from 'vega';
import {isString, toSet} from 'vega-util';
import {isString} from 'vega-util';
import {FieldName} from './channeldef';
import {contains, Flag, keys} from './util';
import {contains, Flag} from './util';

const AGGREGATE_OP_INDEX: Flag<AggregateOp> = {
argmax: 1,
Expand Down Expand Up @@ -55,29 +55,43 @@ export function isArgmaxDef(a: Aggregate | string): a is ArgmaxDef {
return !!a && !!a['argmax'];
}

export const AGGREGATE_OPS = keys(AGGREGATE_OP_INDEX);

export function isAggregateOp(a: string | ArgminDef | ArgmaxDef): a is AggregateOp {
return isString(a) && !!AGGREGATE_OP_INDEX[a];
}

export const COUNTING_OPS: NonArgAggregateOp[] = ['count', 'valid', 'missing', 'distinct'];
export const COUNTING_OPS_INDEX = toSet(COUNTING_OPS);
export const COUNTING_OPS = new Set<NonArgAggregateOp>([
'count',
'valid',
'missing',
'distinct'
]) as ReadonlySet<NonArgAggregateOp>;

export function isCountingAggregateOp(aggregate?: string | Aggregate): boolean {
return isString(aggregate) && COUNTING_OPS_INDEX[aggregate];
return isString(aggregate) && COUNTING_OPS.has(aggregate as any);
}

export function isMinMaxOp(aggregate?: Aggregate | string): boolean {
return isString(aggregate) && contains(['min', 'max'], aggregate);
}

/** Additive-based aggregation operations. These can be applied to stack. */
export const SUM_OPS: NonArgAggregateOp[] = ['count', 'sum', 'distinct', 'valid', 'missing'];
export const SUM_OPS = new Set<NonArgAggregateOp>([
'count',
'sum',
'distinct',
'valid',
'missing'
]) as ReadonlySet<NonArgAggregateOp>;

/**
* Aggregation operators that always produce values within the range [domainMin, domainMax].
*/
export const SHARED_DOMAIN_OPS: AggregateOp[] = ['mean', 'average', 'median', 'q1', 'q3', 'min', 'max'];

export const SHARED_DOMAIN_OP_INDEX = toSet(SHARED_DOMAIN_OPS);
export const SHARED_DOMAIN_OPS = new Set<AggregateOp>([
'mean',
'average',
'median',
'q1',
'q3',
'min',
'max'
]) as ReadonlySet<AggregateOp>;
6 changes: 4 additions & 2 deletions src/compile/scale/domain.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {SignalRef} from 'vega';
import {isObject, isString} from 'vega-util';
import {
Aggregate,
isAggregateOp,
isArgmaxDef,
isArgminDef,
MULTIDOMAIN_SORT_OP_INDEX as UNIONDOMAIN_SORT_OP_INDEX,
NonArgAggregateOp,
SHARED_DOMAIN_OP_INDEX
SHARED_DOMAIN_OPS
} from '../../aggregate';
import {isBinning, isBinParams, isParameterExtent} from '../../bin';
import {getSecondaryRangeChannel, isScaleChannel, ScaleChannel} from '../../channel';
Expand All @@ -22,6 +23,7 @@ import {
valueExpr,
vgField
} from '../../channeldef';
import {CompositeAggregate} from '../../compositemark';
import {DataSourceType} from '../../data';
import {DateTime} from '../../datetime';
import {ExprRef} from '../../expr';
Expand Down Expand Up @@ -505,7 +507,7 @@ export function canUseUnaggregatedDomain(
};
}

if (isString(aggregate) && !SHARED_DOMAIN_OP_INDEX[aggregate]) {
if (isString(aggregate) && !(SHARED_DOMAIN_OPS as Set<Aggregate | CompositeAggregate>).has(aggregate)) {
return {
valid: false,
reason: log.message.unaggregateDomainWithNonSharedDomainOp(aggregate)
Expand Down
2 changes: 1 addition & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ export function initConfig(specifiedConfig: Config = {}): Config<SignalRef> {
return outputConfig;
}

const MARK_STYLES = ['view', ...PRIMITIVE_MARKS] as ('view' | Mark)[];
const MARK_STYLES = new Set(['view', ...PRIMITIVE_MARKS]) as ReadonlySet<'view' | Mark>;

const VL_ONLY_CONFIG_PROPERTIES: (keyof Config)[] = [
'color',
Expand Down
7 changes: 2 additions & 5 deletions src/mark.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {Align, Color, Gradient, MarkConfig as VgMarkConfig, Orientation, SignalRef, TextBaseline} from 'vega';
import {toSet} from 'vega-util';
import {CompositeMark, CompositeMarkDef} from './compositemark';
import {ExprRef} from './expr';
import {Flag, keys} from './util';
Expand Down Expand Up @@ -54,7 +53,7 @@ export function isRectBasedMark(m: Mark | CompositeMark): m is 'rect' | 'bar' |
return ['rect', 'bar', 'image', 'arc' /* arc is rect/interval in polar coordinate */].includes(m);
}

export const PRIMITIVE_MARKS = keys(Mark);
export const PRIMITIVE_MARKS = new Set(keys(Mark));

export interface ColorMixins<ES extends ExprRef | SignalRef> {
/**
Expand Down Expand Up @@ -300,11 +299,9 @@ export function isMarkDef(mark: string | GenericMarkDef<any>): mark is GenericMa
return mark['type'];
}

const PRIMITIVE_MARK_INDEX = toSet(PRIMITIVE_MARKS);

export function isPrimitiveMark(mark: AnyMark): mark is Mark {
const markType = isMarkDef(mark) ? mark.type : mark;
return markType in PRIMITIVE_MARK_INDEX;
return (PRIMITIVE_MARKS as Set<Mark | CompositeMark>).has(markType);
}

export const STROKE_CONFIG = [
Expand Down
69 changes: 34 additions & 35 deletions src/scale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
TimeInterval,
TimeIntervalStep
} from 'vega';
import {isString, toSet} from 'vega-util';
import {isString} from 'vega-util';
import * as CHANNEL from './channel';
import {Channel, isColorChannel} from './channel';
import {DateTime} from './datetime';
Expand Down Expand Up @@ -119,52 +119,64 @@ export function scaleTypePrecedence(scaleType: ScaleType): number {
return SCALE_PRECEDENCE_INDEX[scaleType];
}

export const CONTINUOUS_TO_CONTINUOUS_SCALES: ScaleType[] = ['linear', 'log', 'pow', 'sqrt', 'symlog', 'time', 'utc'];
const CONTINUOUS_TO_CONTINUOUS_INDEX = toSet(CONTINUOUS_TO_CONTINUOUS_SCALES);
export const QUANTITATIVE_SCALES = new Set<ScaleType>([
'linear',
'log',
'pow',
'sqrt',
'symlog'
]) as ReadonlySet<ScaleType>;

export const QUANTITATIVE_SCALES: ScaleType[] = ['linear', 'log', 'pow', 'sqrt', 'symlog'];

const QUANTITATIVE_SCALES_INDEX = toSet(QUANTITATIVE_SCALES);
export const CONTINUOUS_TO_CONTINUOUS_SCALES = new Set<ScaleType>([
...QUANTITATIVE_SCALES,
'time',
'utc'
]) as ReadonlySet<ScaleType>;

export function isQuantitative(type: ScaleType): type is 'linear' | 'log' | 'pow' | 'sqrt' | 'symlog' {
return type in QUANTITATIVE_SCALES_INDEX;
return QUANTITATIVE_SCALES.has(type);
}

export const CONTINUOUS_TO_DISCRETE_SCALES: ScaleType[] = ['quantile', 'quantize', 'threshold'];
const CONTINUOUS_TO_DISCRETE_INDEX = toSet(CONTINUOUS_TO_DISCRETE_SCALES);

export const CONTINUOUS_DOMAIN_SCALES: ScaleType[] = CONTINUOUS_TO_CONTINUOUS_SCALES.concat([
export const CONTINUOUS_TO_DISCRETE_SCALES = new Set<ScaleType>([
'quantile',
'quantize',
'threshold',
'threshold'
]) as ReadonlySet<ScaleType>;

export const CONTINUOUS_DOMAIN_SCALES = new Set<ScaleType>([
...CONTINUOUS_TO_CONTINUOUS_SCALES,
...CONTINUOUS_TO_DISCRETE_SCALES,
'sequential',
'identity'
]);
const CONTINUOUS_DOMAIN_INDEX = toSet(CONTINUOUS_DOMAIN_SCALES);
]) as ReadonlySet<ScaleType>;

export const DISCRETE_DOMAIN_SCALES: ScaleType[] = ['ordinal', 'bin-ordinal', 'point', 'band'];
const DISCRETE_DOMAIN_INDEX = toSet(DISCRETE_DOMAIN_SCALES);
export const DISCRETE_DOMAIN_SCALES = new Set<ScaleType>([
'ordinal',
'bin-ordinal',
'point',
'band'
]) as ReadonlySet<ScaleType>;

export const TIME_SCALE_TYPES: ScaleType[] = ['time', 'utc'];
export const TIME_SCALE_TYPES = new Set<ScaleType>(['time', 'utc']) as ReadonlySet<ScaleType>;

export function hasDiscreteDomain(type: ScaleType): type is 'ordinal' | 'bin-ordinal' | 'point' | 'band' {
return type in DISCRETE_DOMAIN_INDEX;
return DISCRETE_DOMAIN_SCALES.has(type);
}

export function hasContinuousDomain(
type: ScaleType
): type is 'linear' | 'log' | 'pow' | 'sqrt' | 'symlog' | 'time' | 'utc' | 'quantile' | 'quantize' | 'threshold' {
return type in CONTINUOUS_DOMAIN_INDEX;
return CONTINUOUS_DOMAIN_SCALES.has(type);
}

export function isContinuousToContinuous(
type: ScaleType
): type is 'linear' | 'log' | 'pow' | 'sqrt' | 'symlog' | 'time' | 'utc' {
return type in CONTINUOUS_TO_CONTINUOUS_INDEX;
return CONTINUOUS_TO_CONTINUOUS_SCALES.has(type);
}

export function isContinuousToDiscrete(type: ScaleType): type is 'quantile' | 'quantize' | 'threshold' {
return type in CONTINUOUS_TO_DISCRETE_INDEX;
return CONTINUOUS_TO_DISCRETE_SCALES.has(type);
}

export interface ScaleConfig<ES extends ExprRef | SignalRef> {
Expand Down Expand Up @@ -832,20 +844,7 @@ export function scaleTypeSupportDataType(specifiedType: ScaleType, fieldDefType:
} else if (fieldDefType === TEMPORAL) {
return contains([ScaleType.TIME, ScaleType.UTC, undefined], specifiedType);
} else if (fieldDefType === QUANTITATIVE) {
return contains(
[
ScaleType.LOG,
ScaleType.POW,
ScaleType.SQRT,
ScaleType.SYMLOG,
ScaleType.QUANTILE,
ScaleType.QUANTIZE,
ScaleType.THRESHOLD,
ScaleType.LINEAR,
undefined
],
specifiedType
);
return isQuantitative(specifiedType) || isContinuousToDiscrete(specifiedType) || specifiedType === undefined;
}

return true;
Expand Down
10 changes: 7 additions & 3 deletions src/stack.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {array, isBoolean} from 'vega-util';
import {SUM_OPS} from './aggregate';
import {Aggregate, SUM_OPS} from './aggregate';
import {getSecondaryRangeChannel, NonPositionChannel, NONPOSITION_CHANNELS} from './channel';
import {
channelDefType,
Expand All @@ -13,6 +13,7 @@ import {
TypedFieldDef,
vgField
} from './channeldef';
import {CompositeAggregate} from './compositemark';
import {channelHasField, Encoding, isAggregate} from './encoding';
import * as log from './log';
import {
Expand All @@ -32,7 +33,6 @@ import {
TICK
} from './mark';
import {ScaleType} from './scale';
import {contains} from './util';

const STACK_OFFSET_INDEX = {
zero: 1,
Expand Down Expand Up @@ -257,7 +257,11 @@ export function stack(
}

// Warn if stacking non-summative aggregate
if (isFieldDef(stackedFieldDef) && stackedFieldDef.aggregate && !contains(SUM_OPS, stackedFieldDef.aggregate)) {
if (
isFieldDef(stackedFieldDef) &&
stackedFieldDef.aggregate &&
!(SUM_OPS as Set<Aggregate | CompositeAggregate>).has(stackedFieldDef.aggregate)
domoritz marked this conversation as resolved.
Show resolved Hide resolved
) {
log.warn(log.message.stackNonSummativeAggregate(stackedFieldDef.aggregate));
}

Expand Down
2 changes: 1 addition & 1 deletion test/compile/mark/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('compile/mark/init', () => {
});

it('should return undefined by default for other marks', () => {
const otherMarks = without(PRIMITIVE_MARKS, [POINT, TICK, CIRCLE, SQUARE, GEOSHAPE]);
const otherMarks = without([...PRIMITIVE_MARKS], [POINT, TICK, CIRCLE, SQUARE, GEOSHAPE]);
for (const mark of otherMarks) {
const model = parseUnitModelWithScaleAndLayoutSize({
mark,
Expand Down
5 changes: 2 additions & 3 deletions test/compile/scale/parse.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import {toSet} from 'vega-util';
import {parseScaleCore, parseScales} from '../../../src/compile/scale/parse';
import * as log from '../../../src/log';
import {NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES, SCALE_PROPERTIES} from '../../../src/scale';
import {parseModel, parseModelWithScale, parseUnitModelWithScale, without} from '../../util';

describe('src/compile', () => {
it('NON_TYPE_RANGE_SCALE_PROPERTIES should be SCALE_PROPERTIES without type, domain, and range properties', () => {
expect(toSet(NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES)).toEqual(
toSet(without(SCALE_PROPERTIES, ['type', 'domain', 'range', 'rangeMax', 'rangeMin', 'scheme']))
expect(new Set(NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES)).toEqual(
new Set(without(SCALE_PROPERTIES, ['type', 'domain', 'range', 'rangeMax', 'rangeMin', 'scheme']))
);
});

Expand Down