Skip to content

[9.1] [Investigations] - Improve useFieldBrowser performance (#225859)#227004

Merged
michaelolo24 merged 1 commit intoelastic:9.1from
michaelolo24:backport/9.1/pr-225859
Jul 10, 2025
Merged

[9.1] [Investigations] - Improve useFieldBrowser performance (#225859)#227004
michaelolo24 merged 1 commit intoelastic:9.1from
michaelolo24:backport/9.1/pr-225859

Conversation

@michaelolo24
Copy link
Copy Markdown
Contributor

Backport

This will backport the following commits from main to 9.1:

Questions ?

Please refer to the Backport tool documentation

## Summary

This PR looks to improve the performance of the newly introduced
`useBrowserFields` implementation. The currently used
`DataViewSpec.fields` isn't performant at scale and is not cached.
Existing code is as follows:
[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)
call as seen
[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),
[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),
and finally
[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).
While this may not be a significant issue at a few thousand fields, this
does not scale well as the number of fields reaches the tens to hundreds
of thousands.

[Separate PR](elastic#225726) that moves
away from spec usage to pure `DataView` usage

The `useFieldsBrowser` hook is improved by relying directly on the
DataView rather than the DataViewSpec. And there are improvements to the
original sourcerer fieldBrowser calculation. Currently the use of
`memoizeOne` doesn't actually work as the fields object passed to it is
an object and not an array.

A new fieldBrowserManager was introduced that caches based on the scope.
Originally investigated caching via a `WeakMap` to avoid the scenario
where all 4 (current) scopes cache the exact same DataView, but it
looked like references to previously selected dataViews retained a
strong reference in memory, which often led to all N number of selected
dataViews being retained in the cache. The current approach is safer as
the number of cached entries are capped at
`DataViewManagerScopeName.length`

** Relevant configurations **
`xpack.securitySolution.enableExperimental:
['newDataViewPickerEnabled']`

(cherry picked from commit 9a18757)

# Conflicts:
#	x-pack/solutions/security/plugins/security_solution/public/cases/components/ai_for_soc/table.tsx
@michaelolo24 michaelolo24 added the backport This PR is a backport of another PR label Jul 8, 2025
@michaelolo24 michaelolo24 enabled auto-merge (squash) July 8, 2025 12:24
@michaelolo24 michaelolo24 added the backport This PR is a backport of another PR label Jul 8, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #126 / Stateful Observability - Deployment-agnostic API integration tests SyntheticsAPITests EnableDefaultAlerting deletes (and recreates) the default rule when settings are updated

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 7820 7821 +1

Async chunks

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

id before after diff
securitySolution 9.8MB 9.8MB +1.8KB
Unknown metric groups

References to deprecated APIs

id before after diff
securitySolution 345 348 +3

TOOLBAR_VISIBILITY,
} from '../../../detections/components/alert_summary/table/table';
import { ActionsCell } from '../../../detections/components/alert_summary/table/actions_cell';
import { getDataViewStateFromIndexFields } from '../../../common/containers/source/use_data_view';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the only removed import, the other ones were just moved above and useBrowserFields and DataViewManagerScopeName are added

@agusruidiazgd agusruidiazgd self-requested a review July 10, 2025 12:54
@michaelolo24 michaelolo24 merged commit bd608d3 into elastic:9.1 Jul 10, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants