[XY] Allow multiple split accessors

[VladLasitsa]

Completes part of #127115

Summary

Allows multiple split accessors for one layer. (splitAccessor -> splitAccessors).
Some refactoring in code related to splitAccessors.

Testing notes:

kibana
| selectFilter
| demodata
| head 15
| xyVis seriesType="area" xAccessor="project" accessors={visdimension "percent_uptime" format="percent"} splitAccessors='cost' splitAccessors='age'
| render

[Kuznietsov]

Thanks, Vlad. Code LGTM 👍

Tested locally, works fine. But I could miss some exceptional cases.

[elasticmachine]

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

[VladLasitsa]

@elasticmachine merge upstream

[kibanamachine]

expected head sha didn’t match current head ref.

[flash1293]

I couldn't find any issues with behavior, but the performance is very bad (see comments in the code.

I configured a chart with 200 split series (using multi field top values) and getColorAssignments takes over 13s, most of it spent in getAllSeries.

On top of this I noticed that the normalizeTable function is even slower, but that's a separate issue, I'm going to open a bug issue for that.

[flash1293]

Would splitAccessors[0] be EMPTY_ACCESSOR here or would it be an empty string? In generateSeriesId you are not adding an empty EMPTY_ACCESSOR if there is no split

[VladLasitsa]

I've added that here

[VladLasitsa]

@elasticmachine merge upstream

[flash1293]

Performance is much better, but it's still much slower than necessary. It's a recurring pattern, I found it in two places:

kibana/src/plugins/chart_expressions/expression_xy/public/components/legend_action.tsx

Line 57 in 8e9ba98

...acc,

kibana/src/plugins/chart_expressions/expression_xy/public/helpers/color_assignment.ts

Line 79 in cac1728

return [...new Set([...acc, ...allRowSeries])];

If you need to calculate something over a large array (like rows or all fields in a data view), do not use reduce and spread an accumulator collection (array or set) into a new way to prevent mutation. Instead either use map or forEach and mutate a singe collection in the outer closure of the function. If this is not than, it has an squared complexity over the number of entries in the array (see #135792) which can make the code run for a very long time for large arrays (which do occur in the wild - I've seen cases with 50k rows in a single table and 600k fields in a single data view).

It doesn't matter much for quick smoke tests with a few hundred data points at max, but it's very important to keep this in mind

[VladLasitsa]

Performance is much better, but it's still much slower than necessary. It's a recurring pattern, I found it in two places:

kibana/src/plugins/chart_expressions/expression_xy/public/components/legend_action.tsx

Line 57 in 8e9ba98

...acc,

kibana/src/plugins/chart_expressions/expression_xy/public/helpers/color_assignment.ts

Line 79 in cac1728

return [...new Set([...acc, ...allRowSeries])];

If you need to calculate something over a large array (like rows or all fields in a data view), do not use reduce and spread an accumulator collection (array or set) into a new way to prevent mutation. Instead either use map or forEach and mutate a singe collection in the outer closure of the function. If this is not than, it has an squared complexity over the number of entries in the array (see #135792) which can make the code run for a very long time for large arrays (which do occur in the wild - I've seen cases with 50k rows in a single table and 600k fields in a single data view).

It doesn't matter much for quick smoke tests with a few hundred data points at max, but it's very important to keep this in mind

Got it. Changed reduce to forEach

[VladLasitsa]

@elasticmachine merge upstream

[flash1293]

When using a multi field top values in Lens for breakdown, the legend filter actions aren't shown anymore (filtering by clicking in the chart doesn't work as well)

[VladLasitsa]

When using a multi field top values in Lens for breakdown, the legend filter actions aren't shown anymore (filtering by clicking in the chart doesn't work as well)

Fixed

[flash1293]

Tested and everything seems to work well now, LGTM

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 310fc02
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
expressionXY	102.6KB	103.3KB	+753.0B
lens	1.2MB	1.2MB	+1.0B
total			+754.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
expressionXY	32.8KB	32.8KB	+20.0B

History

• 💚 Build #56714 succeeded d54664a
• 💚 Build #56527 succeeded bf6abe9
• 💚 Build #56198 succeeded ea7c941
• 💚 Build #55903 succeeded 795ef97
• 💛 Build #55866 was flaky cac1728
• 💔 Build #55833 failed 8e9ba98

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @VladLasitsa