Skip to content

Commit

Permalink
Fix multi terms performance bottleneck (#126575) (#126758)
Browse files Browse the repository at this point in the history
(cherry picked from commit 555ec91)

Co-authored-by: Joe Reuter <[email protected]>
  • Loading branch information
kibanamachine and flash1293 authored Mar 3, 2022
1 parent d164f21 commit 4aaeda2
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe('getAggsFormats', () => {
const mapping = {
id: 'multi_terms',
params: {
paramsPerField: Array(terms.length).fill({ id: 'terms' }),
paramsPerField: [{ id: 'terms' }, { id: 'terms' }, { id: 'terms' }],
},
};

Expand All @@ -141,7 +141,7 @@ describe('getAggsFormats', () => {
const mapping = {
id: 'multi_terms',
params: {
paramsPerField: Array(terms.length).fill({ id: 'terms' }),
paramsPerField: [{ id: 'terms' }, { id: 'terms' }, { id: 'terms' }],
separator: ' - ',
},
};
Expand Down
16 changes: 13 additions & 3 deletions src/plugins/data/common/search/aggs/utils/get_aggs_formats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { i18n } from '@kbn/i18n';
import {
FieldFormat,
FieldFormatInstanceType,
FieldFormatParams,
FieldFormatsContentType,
IFieldFormat,
SerializedFieldFormat,
Expand Down Expand Up @@ -133,11 +134,20 @@ export function getAggsFormats(getFieldFormat: GetFieldFormat): FieldFormatInsta
static id = 'multi_terms';
static hidden = true;

private formatCache: Map<SerializedFieldFormat<FieldFormatParams>, FieldFormat> = new Map();

convert = (val: unknown, type: FieldFormatsContentType) => {
const params = this._params;
const formats = (params.paramsPerField as SerializedFieldFormat[]).map((fieldParams) =>
getFieldFormat({ id: fieldParams.id, params: fieldParams })
);
const formats = (params.paramsPerField as SerializedFieldFormat[]).map((fieldParams) => {
const isCached = this.formatCache.has(fieldParams);
const cachedFormat =
this.formatCache.get(fieldParams) ||
getFieldFormat({ id: fieldParams.id, params: fieldParams });
if (!isCached) {
this.formatCache.set(fieldParams, cachedFormat);
}
return cachedFormat;
});

if (String(val) === '__other__') {
return params.otherBucketLabel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,15 @@ export function getColorAssignments(
}
const splitAccessor = layer.splitAccessor;
const column = data.tables[layer.layerId]?.columns.find(({ id }) => id === splitAccessor);
const columnFormatter = column && formatFactory(column.meta.params);
const splits =
!column || !data.tables[layer.layerId]
? []
: uniq(
data.tables[layer.layerId].rows.map((row) => {
let value = row[splitAccessor];
if (value && !isPrimitive(value)) {
value = formatFactory(column.meta.params).convert(value);
value = columnFormatter?.convert(value) ?? value;
} else {
value = String(value);
}
Expand Down
18 changes: 13 additions & 5 deletions x-pack/plugins/lens/public/xy_visualization/expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@ import type {
ExpressionRenderDefinition,
Datatable,
DatatableRow,
DatatableColumn,
} from 'src/plugins/expressions/public';
import { IconType } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { RenderMode } from 'src/plugins/expressions';
import { ThemeServiceStart } from 'kibana/public';
import { FieldFormat } from 'src/plugins/field_formats/common';
import { EmptyPlaceholder } from '../../../../../src/plugins/charts/public';
import { KibanaThemeProvider } from '../../../../../src/plugins/kibana_react/public';
import type { ILensInterpreterRenderHandlers, LensFilterEvent, LensBrushEvent } from '../types';
Expand Down Expand Up @@ -608,6 +610,7 @@ export function XYChart({
: undefined,
},
};

return (
<Chart ref={chartRef}>
<Settings
Expand Down Expand Up @@ -737,6 +740,11 @@ export function XYChart({

const table = data.tables[layerId];

const formatterPerColumn = new Map<DatatableColumn, FieldFormat>();
for (const column of table.columns) {
formatterPerColumn.set(column, formatFactory(column.meta.params));
}

// what if row values are not primitive? That is the case of, for instance, Ranges
// remaps them to their serialized version with the formatHint metadata
// In order to do it we need to make a copy of the table as the raw one is required for more features (filters, etc...) later on
Expand All @@ -751,7 +759,7 @@ export function XYChart({
// pre-format values for ordinal x axes because there can only be a single x axis formatter on chart level
(!isPrimitive(record) || (column.id === xAccessor && xScaleType === 'ordinal'))
) {
newRow[column.id] = formatFactory(column.meta.params).convert(record);
newRow[column.id] = formatterPerColumn.get(column)!.convert(record);
}
}
return newRow;
Expand Down Expand Up @@ -805,6 +813,8 @@ export function XYChart({
);

const formatter = table?.columns.find((column) => column.id === accessor)?.meta?.params;
const splitHint = table.columns.find((col) => col.id === splitAccessor)?.meta?.params;
const splitFormatter = formatFactory(splitHint);

const seriesProps: SeriesSpec = {
splitSeriesAccessors: splitAccessor ? [splitAccessor] : [],
Expand Down Expand Up @@ -864,8 +874,6 @@ export function XYChart({
},
},
name(d) {
const splitHint = table.columns.find((col) => col.id === splitAccessor)?.meta?.params;

// For multiple y series, the name of the operation is used on each, either:
// * Key - Y name
// * Formatted value - Y name
Expand All @@ -878,7 +886,7 @@ export function XYChart({
splitAccessor &&
!layersAlreadyFormatted[splitAccessor]
) {
return formatFactory(splitHint).convert(key);
return splitFormatter.convert(key);
}
return splitAccessor && i === 0 ? key : columnToLabelMap[key] ?? '';
})
Expand All @@ -892,7 +900,7 @@ export function XYChart({
if (splitAccessor && layersAlreadyFormatted[splitAccessor]) {
return d.seriesKeys[0];
}
return formatFactory(splitHint).convert(d.seriesKeys[0]);
return splitFormatter.convert(d.seriesKeys[0]);
}
// This handles both split and single-y cases:
// * If split series without formatting, show the value literally
Expand Down

0 comments on commit 4aaeda2

Please sign in to comment.