Improve Performance in Row Grouping

[bkorobeinikov]

There is a performance bottleneck when using the row grouping feature. It is noticeable with a large number of rows and about >~20 groups or so.

Profiler showed that the call to mapRowIndexToPath during row height resolution takes most of the time, and it gets slower scrolling towards the end of the list.

rowIndex property was added to identify the exact position of the group in the list. This allowed to create a map that then could be looked up to identify whether the row is a group row or not and resolve the height accordingly, avoiding the expensive call to mapRowIndexToPath.

For further performance improvements, with rowIndex property functions like rowNumberMapperOut and mapRowIndexToPath could be changed to do a binary search, because mapRowIndexToPath can be used as mapper function in the getCellContent consumer code.

scrolling profiler recordings:

before change

after

related to #823

[theiliad]

@jassmith I'm curious if this has been reviewed? I'm also having issues with large resource clogs when using row grouping

[Copilot]

Pull Request Overview

This PR adds a fast lookup for group rows by introducing a rowIndex in each flattened group, replaces expensive path mapping in height/theming calcs with a map lookup, updates tests to assert the new behavior, and scales the example story to demonstrate performance at high row counts.

• Added rowIndex to each FlattenedRowGroup and built a flattenedRowGroupsMap for O(1) row-header checks
• Updated rowHeight and getRowThemeOverride to use the new map instead of mapRowIndexToPath for better scrolling perf
• Extended tests for grouping hooks and expanded the docs story to simulate 100 000 rows with many groups

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
packages/core/src/data-editor/row-grouping.ts	Introduced rowIndex, built a lookup map, switched rowHeight to map-based logic
packages/core/test/row-grouping.test.ts	Added rowIndex assertions and new tests for theming and heights
packages/core/src/docs/examples/row-grouping.stories.tsx	Scaled story to 100 000 rows and auto-generated groups

Comments suppressed due to low confidence (4)

packages/core/src/docs/examples/row-grouping.stories.tsx:37

• The story now renders 100 000 rows but still generates mock data for only 100 rows. Consider passing rows into useMockDataGenerator so that cell lookups beyond the first 100 don’t return undefined.

const { cols, getCellContent } = useMockDataGenerator(100);

packages/core/test/row-grouping.test.ts:237

• We have tests for non-group-header theming but no assertions for group headers (should return options.themeOverride or undefined). Adding a test for a known header row will lock in the intended behavior.

it("returns correct theme for non-group-header rows when some groups collapsed according to getRowThemeOverrideIn", () => {

packages/core/test/row-grouping.test.ts:355

• There are no tests for rowNumberMapper output. Adding cases to verify mapping from visible row index to actual data index will ensure correct behavior when groups are collapsed or expanded.

describe("useRowGroupingInner - rows", () => {

packages/core/src/data-editor/row-grouping.ts:257

• [nitpick] The name flattenedRowGroupsMap is a bit generic. Renaming to something like rowGroupByRowIndex or rowIndexToGroupMap could improve readability and clarify that keys are row indices.

const flattenedRowGroupsMap = React.useMemo(() => {

[lukasmasuch]

LGTM 👍