From 697308aa434c3e29549d067021604507f8ac017e Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 1 Mar 2022 13:21:57 +0100 Subject: [PATCH 1/3] fix performance bottleneck --- .../search/aggs/utils/get_aggs_formats.ts | 16 +++++++++++++--- .../xy_visualization/color_assignment.ts | 3 ++- .../public/xy_visualization/expression.tsx | 18 +++++++++++++----- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/plugins/data/common/search/aggs/utils/get_aggs_formats.ts b/src/plugins/data/common/search/aggs/utils/get_aggs_formats.ts index f14f981fdec65..e514cc24f93ca 100644 --- a/src/plugins/data/common/search/aggs/utils/get_aggs_formats.ts +++ b/src/plugins/data/common/search/aggs/utils/get_aggs_formats.ts @@ -12,6 +12,7 @@ import { i18n } from '@kbn/i18n'; import { FieldFormat, FieldFormatInstanceType, + FieldFormatParams, FieldFormatsContentType, IFieldFormat, SerializedFieldFormat, @@ -133,11 +134,20 @@ export function getAggsFormats(getFieldFormat: GetFieldFormat): FieldFormatInsta static id = 'multi_terms'; static hidden = true; + private formatCache: Map, 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; diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index e1e2ba75b50c4..f94a51e7efb75 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -59,6 +59,7 @@ 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] ? [] @@ -66,7 +67,7 @@ export function getColorAssignments( 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); } diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.tsx index ea0e336ff2f08..fb8ad127a394e 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.tsx @@ -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'; @@ -599,6 +601,7 @@ export function XYChart({ : undefined, }, }; + return ( (); + 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 @@ -744,7 +752,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; @@ -798,6 +806,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] : [], @@ -857,8 +867,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 @@ -871,7 +879,7 @@ export function XYChart({ splitAccessor && !layersAlreadyFormatted[splitAccessor] ) { - return formatFactory(splitHint).convert(key); + return splitFormatter.convert(key); } return splitAccessor && i === 0 ? key : columnToLabelMap[key] ?? ''; }) @@ -885,7 +893,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 From 030f9127b86d3359f2c7b938dac922e3e712fb9b Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 1 Mar 2022 15:46:13 +0100 Subject: [PATCH 2/3] fix tests --- .../data/common/search/aggs/utils/get_aggs_formats.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/data/common/search/aggs/utils/get_aggs_formats.test.ts b/src/plugins/data/common/search/aggs/utils/get_aggs_formats.test.ts index 8510acf1572c7..963fe024c1f8f 100644 --- a/src/plugins/data/common/search/aggs/utils/get_aggs_formats.test.ts +++ b/src/plugins/data/common/search/aggs/utils/get_aggs_formats.test.ts @@ -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' }], }, }; @@ -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: ' - ', }, }; From c1e502fc0b3d4d30a52d598d632621e005c366c1 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 1 Mar 2022 17:51:06 +0100 Subject: [PATCH 3/3] Update x-pack/plugins/lens/public/xy_visualization/color_assignment.ts Co-authored-by: Marco Liberati --- x-pack/plugins/lens/public/xy_visualization/color_assignment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index f94a51e7efb75..2c56725438421 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -67,7 +67,7 @@ export function getColorAssignments( data.tables[layer.layerId].rows.map((row) => { let value = row[splitAccessor]; if (value && !isPrimitive(value)) { - value = columnFormatter?.convert(value) || value; + value = columnFormatter?.convert(value) ?? value; } else { value = String(value); }