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.

Sign up for GitHub

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

Optimize reduce loops - late 2023 edition #168812

Closed
wants to merge 17 commits into from
Closed

Optimize reduce loops - late 2023 edition #168812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
bf5dcf6
optimize reduce loops
pgayvallet Oct 13, 2023
52d553e
Some other optims
pgayvallet Oct 13, 2023
ab7ac12
moar
pgayvallet Oct 13, 2023
8f7d129
moar
pgayvallet Oct 13, 2023
4b2c4fe
moar
pgayvallet Oct 13, 2023
91f706d
moar
pgayvallet Oct 13, 2023
30bb31a
moar
pgayvallet Oct 13, 2023
35aeea4
moar
pgayvallet Oct 13, 2023
7ebde8e
Merge remote-tracking branch 'upstream/main' into kbn-xxx-reduce-opti…
pgayvallet Oct 13, 2023
264c2db
moar
pgayvallet Oct 14, 2023
605a66b
moar
pgayvallet Oct 15, 2023
4b1067d
moar
pgayvallet Oct 15, 2023
c71383a
moar
pgayvallet Oct 15, 2023
7cc32bd
moar
pgayvallet Oct 15, 2023
b2108b1
fix types
pgayvallet Oct 15, 2023
b30e195
revert array/set conversion
pgayvallet Oct 16, 2023
18600e4
fix things
pgayvallet Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions ...-objects/core-saved-objects-api-server-internal/src/lib/search/aggregations/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,14 @@ const validateAggregationContainer = (
) => {
return Object.entries(container).reduce<estypes.AggregationsAggregationContainer>(
(memo, [aggName, aggregation]) => {
if (aggregationKeys.includes(aggName)) {
return memo;
if (!aggregationKeys.includes(aggName)) {
(memo as Record<string, unknown>)[aggName] = validateAggregationType(
aggName,
aggregation,
childContext(context, aggName)
);
}
return {
...memo,
[aggName]: validateAggregationType(aggName, aggregation, childContext(context, aggName)),
};
return memo;
},
{}
);
Expand Down
10 changes: 2 additions & 8 deletions packages/kbn-config-schema/src/types/object_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,7 @@ export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>>
...newProps,
}).reduce((memo, [key, value]) => {
if (value !== null && value !== undefined) {
return {
...memo,
[key]: value,
};
(memo as Record<string, unknown>)[key] = value;
}
return memo;
}, {} as ExtendedProps<P, NP>);
Expand All @@ -178,10 +175,7 @@ export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>>
public extendsDeep(options: ExtendsDeepOptions) {
const extendedProps = Object.entries(this.props).reduce((memo, [key, value]) => {
if (value !== null && value !== undefined) {
return {
...memo,
[key]: value.extendsDeep(options),
};
Object.assign(memo, { [key]: value.extendsDeep(options) });
}
return memo;
}, {} as P);
Expand Down
8 changes: 4 additions & 4 deletions packages/kbn-custom-integrations/src/state_machines/create/pipelines/fields.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ const updateTouchedFields =
...context,
touchedFields: {
...context.touchedFields,
...Object.keys(event.fields).reduce<WithTouchedFields['touchedFields']>(
(acc, field) => ({ ...acc, [field]: true }),
{} as WithTouchedFields['touchedFields']
),
...Object.keys(event.fields).reduce<WithTouchedFields['touchedFields']>((acc, field) => {
acc[field as keyof WithTouchedFields['touchedFields']] = true;
return acc;
}, {} as WithTouchedFields['touchedFields']),
},
};
return [mergedContext, event];
Expand Down
29 changes: 11 additions & 18 deletions packages/kbn-es-query/src/kuery/functions/utils/get_full_field_name_node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,17 @@ export function getFullFieldNameNode(
const nestedPathFromField = subTypeNested?.nested.path;

if (nestedPath && !nestedPathFromField) {
return [
...acc,
`${field.name} is not a nested field but is in nested group "${nestedPath}" in the KQL expression.`,
];
}

if (nestedPathFromField && !nestedPath) {
return [
...acc,
`${field.name} is a nested field, but is not in a nested group in the KQL expression.`,
];
}

if (nestedPathFromField !== nestedPath) {
return [
...acc,
`Nested field ${field.name} is being queried with the incorrect nested path. The correct path is ${subTypeNested?.nested.path}.`,
];
acc.push(
`${field.name} is not a nested field but is in nested group "${nestedPath}" in the KQL expression.`
);
} else if (nestedPathFromField && !nestedPath) {
acc.push(
`${field.name} is a nested field, but is not in a nested group in the KQL expression.`
);
} else if (nestedPathFromField !== nestedPath) {
acc.push(
`Nested field ${field.name} is being queried with the incorrect nested path. The correct path is ${subTypeNested?.nested.path}.`
);
}

return acc;
Expand Down
15 changes: 5 additions & 10 deletions packages/kbn-i18n/src/core/helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,12 @@ export const unique = <T>(arr: T[] = []): T[] => [...new Set(arr)];
const merge = (a: any, b: any): { [k: string]: any } =>
unique([...Object.keys(a), ...Object.keys(b)]).reduce((acc, key) => {
if (isObject(a[key]) && isObject(b[key]) && !Array.isArray(a[key]) && !Array.isArray(b[key])) {
return {
...acc,
[key]: merge(a[key], b[key]),
};
acc[key] = merge(a[key], b[key]);
} else {
acc[key] = b[key] === undefined ? a[key] : b[key];
}

return {
...acc,
[key]: b[key] === undefined ? a[key] : b[key],
};
}, {});
return acc;
}, {} as { [k: string]: any });

export const mergeAll = (...sources: any[]) =>
sources.filter(isObject).reduce((acc, source) => merge(acc, source));
11 changes: 4 additions & 7 deletions packages/kbn-i18n/src/loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,10 @@ export async function getAllTranslations(): Promise<{ [key: string]: Translation
const locales = getRegisteredLocales();
const translations = await Promise.all(locales.map(getTranslationsByLocale));

return locales.reduce(
(acc, locale, index) => ({
...acc,
[locale]: translations[index],
}),
{}
);
return locales.reduce((acc, locale, index) => {
acc[locale] = translations[index];
return acc;
}, {} as { [key: string]: Translation });
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/kbn-json-ast/src/snip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ export function snip(source: string, snips: Snip[]) {
.reduce((acc: Snip[], s) => {
const prev = acc.at(-1);
if (!prev || prev[1] < s[0]) {
return [...acc, s];
acc.push(s);
return acc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a similar spread a few lines below. In the light of total elimination, should that be replaced by something like:

      const merged: Snip = [Math.min(prev[0], s[0]), Math.max(prev[1], s[1])];
      acc[acc.length-1] = merged;
      return acc;

}

if (prev[2] || s[2]) {
Expand Down
12 changes: 7 additions & 5 deletions packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@ export const getEsQueryConfig = (params?: GetEsQueryConfigParamType): EsQueryCon
if (params == null) {
return defaultConfigValues;
}
const paramKeysWithValues = Object.keys(params).reduce((acc: EsQueryConfig, key) => {
const paramKeysWithValues = Object.keys(params).reduce((acc, key) => {
const configKey = key as ConfigKeys;
if (params[configKey] != null) {
return { [key]: params[configKey], ...acc };
acc[key] = params[configKey];
} else {
acc[key] = defaultConfigValues[configKey];
}
return { [key]: defaultConfigValues[configKey], ...acc };
}, {} as EsQueryConfig);
return paramKeysWithValues;
return acc;
}, {} as Record<string, unknown>);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

castings like this in reduce can be avoided by passing interface in reduce generic

  const paramKeysWithValues = Object.keys(params).reduce<Record<string, unknown>>((acc, key) => {

return paramKeysWithValues as EsQueryConfig;
};
11 changes: 6 additions & 5 deletions packages/kbn-securitysolution-grouping/src/containers/query/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { Filter, FILTERS } from '@kbn/es-query';

export const getEmptyValue = () => '—';

type StrictFilter = Filter & {
Expand All @@ -19,9 +20,8 @@ export const createGroupFilter = (
): StrictFilter[] =>
values != null && values.length > 0
? values.reduce(
(acc: StrictFilter[], query) => [
...acc,
{
(acc: StrictFilter[], query) => {
acc.push({
meta: {
alias: null,
disabled: false,
Expand All @@ -39,8 +39,9 @@ export const createGroupFilter = (
},
},
},
},
],
});
return acc;
},
[
{
meta: {
Expand Down
9 changes: 5 additions & 4 deletions packages/kbn-securitysolution-list-utils/src/get_ids_and_namespaces/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ export const getIdsAndNamespaces = ({
}
})
.reduce<{ ids: string[]; namespaces: NamespaceType[] }>(
(acc, { listId, namespaceType }) => ({
ids: [...acc.ids, listId],
namespaces: [...acc.namespaces, namespaceType],
}),
(acc, { listId, namespaceType }) => {
acc.ids.push(listId);
acc.namespaces.push(namespaceType);
return acc;
},
{ ids: [], namespaces: [] }
);
3 changes: 2 additions & 1 deletion packages/kbn-securitysolution-list-utils/src/helpers/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,8 @@ export const getFormattedBuilderEntries = (
parentIndex,
allowCustomFieldOptions
);
return [...acc, newItemEntry];
acc.push(newItemEntry);
return acc;
} else {
const parentEntry: FormattedBuilderEntry = {
correspondingKeywordField: undefined,
Expand Down
5 changes: 4 additions & 1 deletion packages/kbn-std/src/iteration/map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,8 @@ export async function asyncMapWithLimit<T1, T2>(

return results
.sort(([a], [b]) => a - b)
.reduce((acc: T2[], [, result]) => acc.concat(result), []);
.reduce((acc: T2[], [, result]) => {
acc.push(...result);
return acc;
}, []);
Comment on lines +61 to +64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wonder if flatMap would do the same...

hmmm, nevermind:
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold on... that was on Chrome...

Safari
image

And Firefox
image

Are consistently faster 🤷

Copy link
Member

@afharo afharo Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running a benchmark on NodeJS shows reduce with push is best

flatMap x 1,226,284 ops/sec ±0.40% (97 runs sampled)
reduce with concat x 6,033,030 ops/sec ±0.19% (101 runs sampled)
reduce with push x 16,624,071 ops/sec ±0.36% (93 runs sampled)
Fastest is reduce with push

This is the piece of code I ran:

const Benchmark = require('benchmark');

const suite = new Benchmark.Suite;

const results = [
  [0, ["a", "b", "c"]],
  [2, ["g", "h", "i"]],
  [1, ["d", "e", "f"]],
].sort(([a], [b]) => a - b);

suite
  .add('flatMap', function () {
    const out1 = results.flatMap(([, result]) => result);
  })
  .add('reduce with concat', function () {
    const out1 = results.reduce((acc, [, result]) => acc.concat(result), []);
  })
  .add('reduce with push', function () {
    const out1 = results.reduce((acc, [, result]) => {
      acc.push(...result);
      return acc;
    }, []);
  });

suite
  // add listeners
  .on('cycle', function (event) {
    console.log(String(event.target));
  })
  .on('complete', function () {
    console.log('Fastest is ' + this.filter('fastest').map('name'));
  });

suite.run();

}
5 changes: 3 additions & 2 deletions packages/kbn-telemetry-tools/src/tools/serializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ export function getConstraints(node: ts.Node, program: ts.Program): any {
return types.reduce<any>((acc, typeNode) => {
const constraints = getConstraints(typeNode, program);
const contraintsArray = Array.isArray(constraints) ? constraints : [constraints];
return [...acc, ...contraintsArray];
acc.push(...contraintsArray);
return acc;
}, []);
}

Expand Down Expand Up @@ -178,7 +179,7 @@ export function getDescriptor(node: ts.Node, program: ts.Program): Descriptor |
const constraints = getConstraints(constraint, program);
const constraintsArray = Array.isArray(constraints) ? constraints : [constraints];
if (typeof constraintsArray[0] === 'string') {
return constraintsArray.reduce((acc, c) => ({ ...acc, [c]: descriptor }), {});
return constraintsArray.reduce((acc, c) => Object.assign(acc, { [c]: descriptor }), {});
}
}
return { '@@INDEX@@': descriptor };
Expand Down
8 changes: 4 additions & 4 deletions packages/kbn-test/src/kbn_archiver_cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ function parseTypesFlag(flags: Flags) {
}

const types = typeof flags.type === 'string' ? [flags.type] : flags.type;
return types.reduce(
(acc: string[], type) => [...acc, ...type.split(',').map((t) => t.trim())],
[]
);
return types.reduce((acc: string[], type) => {
acc.push(...type.split(',').map((t) => t.trim()));
return acc;
}, []);
}

export function runKbnArchiverCli() {
Expand Down
6 changes: 3 additions & 3 deletions ...ins/chart_expressions/expression_xy/common/expression_functions/common_data_layer_args.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ export const commonDataLayerArgs: Omit<
seriesType: {
aliases: ['_'],
types: ['string'],
options: [...Object.values(SeriesTypes)],
options: Object.values(SeriesTypes),
help: strings.getSeriesTypeHelp(),
required: true,
strict: true,
},
xScaleType: {
options: [...Object.values(XScaleTypes)],
options: Object.values(XScaleTypes),
help: strings.getXScaleTypeHelp(),
strict: true,
},
Expand All @@ -60,7 +60,7 @@ export const commonDataLayerArgs: Omit<
},
curveType: {
types: ['string'],
options: [...Object.values(XYCurveTypes)],
options: Object.values(XYCurveTypes),
help: strings.getCurveTypeHelp(),
strict: true,
},
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/chart_expressions/expression_xy/common/expression_functions/common_xy_args.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ export const commonXYArgs: CommonXYFn['args'] = {
},
fittingFunction: {
types: ['string'],
options: [...Object.values(FittingFunctions)],
options: Object.values(FittingFunctions),
help: strings.getFittingFunctionHelp(),
strict: true,
},
endValue: {
types: ['string'],
options: [...Object.values(EndValues)],
options: Object.values(EndValues),
help: strings.getEndValueHelp(),
strict: true,
},
Expand All @@ -44,7 +44,7 @@ export const commonXYArgs: CommonXYFn['args'] = {
},
valueLabels: {
types: ['string'],
options: [...Object.values(ValueLabelModes)],
options: Object.values(ValueLabelModes),
help: strings.getValueLabelsHelp(),
strict: true,
default: ValueLabelModes.HIDE,
Expand Down
8 changes: 4 additions & 4 deletions src/plugins/chart_expressions/expression_xy/common/expression_functions/reference_line.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const referenceLineFunction: ReferenceLineFn = {
},
lineStyle: {
types: ['string'],
options: [...Object.values(LineStyles)],
options: Object.values(LineStyles),
help: i18n.translate('expressionXY.decorationConfig.lineStyle.help', {
defaultMessage: 'The style of the reference line',
}),
Expand All @@ -78,12 +78,12 @@ export const referenceLineFunction: ReferenceLineFn = {
help: i18n.translate('expressionXY.decorationConfig.icon.help', {
defaultMessage: 'An optional icon used for reference lines',
}),
options: [...Object.values(AvailableReferenceLineIcons)],
options: Object.values(AvailableReferenceLineIcons),
strict: true,
},
iconPosition: {
types: ['string'],
options: [...Object.values(IconPositions)],
options: Object.values(IconPositions),
help: i18n.translate('expressionXY.decorationConfig.iconPosition.help', {
defaultMessage: 'The placement of the icon for the reference line',
}),
Expand All @@ -98,7 +98,7 @@ export const referenceLineFunction: ReferenceLineFn = {
},
fill: {
types: ['string'],
options: [...Object.values(FillStyles)],
options: Object.values(FillStyles),
help: i18n.translate('expressionXY.decorationConfig.fill.help', {
defaultMessage: 'Fill',
}),
Expand Down
8 changes: 4 additions & 4 deletions ...expressions/expression_xy/common/expression_functions/reference_line_decoration_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const referenceLineDecorationConfigFunction: ReferenceLineDecorationConfi
},
lineStyle: {
types: ['string'],
options: [...Object.values(LineStyles)],
options: Object.values(LineStyles),
help: i18n.translate('expressionXY.decorationConfig.lineStyle.help', {
defaultMessage: 'The style of the reference line',
}),
Expand All @@ -56,12 +56,12 @@ export const referenceLineDecorationConfigFunction: ReferenceLineDecorationConfi
help: i18n.translate('expressionXY.decorationConfig.icon.help', {
defaultMessage: 'An optional icon used for reference lines',
}),
options: [...Object.values(AvailableReferenceLineIcons)],
options: Object.values(AvailableReferenceLineIcons),
strict: true,
},
iconPosition: {
types: ['string'],
options: [...Object.values(IconPositions)],
options: Object.values(IconPositions),
help: i18n.translate('expressionXY.decorationConfig.iconPosition.help', {
defaultMessage: 'The placement of the icon for the reference line',
}),
Expand All @@ -75,7 +75,7 @@ export const referenceLineDecorationConfigFunction: ReferenceLineDecorationConfi
},
fill: {
types: ['string'],
options: [...Object.values(FillStyles)],
options: Object.values(FillStyles),
help: i18n.translate('expressionXY.decorationConfig.fill.help', {
defaultMessage: 'Fill',
}),
Expand Down
Loading