Skip to content

[Investigations] - Improve useFieldBrowser performance#225859

Merged
michaelolo24 merged 5 commits intoelastic:mainfrom
michaelolo24:use-browser-fields-performance
Jul 7, 2025
Merged

[Investigations] - Improve useFieldBrowser performance#225859
michaelolo24 merged 5 commits intoelastic:mainfrom
michaelolo24:use-browser-fields-performance

Conversation

@michaelolo24
Copy link
Copy Markdown
Contributor

@michaelolo24 michaelolo24 commented Jun 30, 2025

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 call as seen here, here, and finally here. 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 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']

@michaelolo24 michaelolo24 force-pushed the use-browser-fields-performance branch from e23f816 to 22fd768 Compare June 30, 2025 17:26
@michaelolo24 michaelolo24 added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Jun 30, 2025
@michaelolo24 michaelolo24 force-pushed the use-browser-fields-performance branch 4 times, most recently from 01816ca to 030082b Compare July 2, 2025 18:57
@michaelolo24 michaelolo24 marked this pull request as ready for review July 2, 2025 19:04
@michaelolo24 michaelolo24 requested review from a team as code owners July 2, 2025 19:04
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

@michaelolo24 michaelolo24 requested a review from umbopepato July 2, 2025 19:04
@michaelolo24 michaelolo24 force-pushed the use-browser-fields-performance branch from 6407cdd to aa88062 Compare July 3, 2025 12:44
const { browserFields } = useMemo(
() => getDataViewStateFromIndexFields('', dataViewSpec.fields),
[dataViewSpec.fields]
() => browserFieldsManager.getBrowserFields(dataView, DataViewManagerScopeName.detections),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like we should encourage developers to use the useBrowserFields hook instead of using the browserFieldsManager directly, unless there is a reason not to?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we'll make sure a bit more required once we move all the content of the data_view_manager folder into a package, but I think we should start using the correct files now if we can?

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.

Yea, only problem is: useBrowserFields currently only properly works when the flag is on. useDataView doesn't return anything when the flag is off 😅 , so this would break everything. I'd rather not add custom logic to useBrowserFields, but can add a comment to these to replace them when the flag is defaulted on?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it better to now add custom logic to useBrowserFields or to add logic that will have to be removed every where it's being used? 😄

@michaelolo24 michaelolo24 force-pushed the use-browser-fields-performance branch from aa88062 to 461fa85 Compare July 4, 2025 23:53
Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

LGTM! Left a few non-blocker comments, mostly cleanup and somewhat subjective so feel free to ignore if you disagree with them :)

@michaelolo24 michaelolo24 force-pushed the use-browser-fields-performance branch from 461fa85 to d2b9fa3 Compare July 7, 2025 17:32
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 7853 7854 +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

History

Copy link
Copy Markdown
Contributor

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement @michaelolo24 🙏
✅ Desk tested locally
LGTM 🚀

@michaelolo24 michaelolo24 merged commit 9a18757 into elastic:main Jul 7, 2025
12 checks passed
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.19, 9.1

https://github.com/elastic/kibana/actions/runs/16127071487

@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.19, 9.1

https://github.com/elastic/kibana/actions/runs/16127075971

@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
8.19 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.19:
- [Security Solution][AI4DSOC] Fix table not applying alert tags for Attack discovery and Cases pages in AI4DSOC (#219410)
- [AI4DSOC] Add checkboxes to the alert summary table (#219169)
9.1 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 225859

Questions ?

Please refer to the Backport tool documentation

1 similar comment
@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
8.19 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.19:
- [Security Solution][AI4DSOC] Fix table not applying alert tags for Attack discovery and Cases pages in AI4DSOC (#219410)
- [AI4DSOC] Add checkboxes to the alert summary table (#219169)
9.1 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 225859

Questions ?

Please refer to the Backport tool documentation

michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Jul 8, 2025
## 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
Copy link
Copy Markdown
Contributor Author

💚 All backports created successfully

Status Branch Result
9.1
8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Jul 8, 2025
## 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
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 8, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @michaelolo24

@kibanamachine
Copy link
Copy Markdown
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.
cc: @michaelolo24

michaelolo24 added a commit that referenced this pull request Jul 10, 2025
… (#227009)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Investigations] - Improve useFieldBrowser performance
(#225859)](#225859)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Michael
Olorunnisola","email":"michael.olorunnisola@elastic.co"},"sourceCommit":{"committedDate":"2025-07-07T20:23:14Z","message":"[Investigations]
- Improve useFieldBrowser performance (#225859)\n\n## Summary\n\nThis PR
looks to improve the performance of the newly
introduced\n`useBrowserFields` implementation. The currently
used\n`DataViewSpec.fields` isn't performant at scale and is not
cached.\nExisting code is as
follows:\n[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)\ncall
as
seen\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),\nand
finally\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).\nWhile
this may not be a significant issue at a few thousand fields, this\ndoes
not scale well as the number of fields reaches the tens to hundreds\nof
thousands.\n\n[Separate
PR](#225726) that moves\naway from
spec usage to pure `DataView` usage\n\nThe `useFieldsBrowser` hook is
improved by relying directly on the\nDataView rather than the
DataViewSpec. And there are improvements to the\noriginal sourcerer
fieldBrowser calculation. Currently the use of\n`memoizeOne` doesn't
actually work as the fields object passed to it is\nan object and not an
array.\n\nA new fieldBrowserManager was introduced that caches based on
the scope.\nOriginally investigated caching via a `WeakMap` to avoid the
scenario\nwhere all 4 (current) scopes cache the exact same DataView,
but it\nlooked like references to previously selected dataViews retained
a\nstrong reference in memory, which often led to all N number of
selected\ndataViews being retained in the cache. The current approach is
safer as\nthe number of cached entries are capped
at\n`DataViewManagerScopeName.length`\n\n\n** Relevant configurations
**\n`xpack.securitySolution.enableExperimental:\n['newDataViewPickerEnabled']`","sha":"9a18757162b038898f5dfb11f1aa3a1130ac749a","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat
Hunting:Investigations","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[Investigations]
- Improve useFieldBrowser
performance","number":225859,"url":"https://github.com/elastic/kibana/pull/225859","mergeCommit":{"message":"[Investigations]
- Improve useFieldBrowser performance (#225859)\n\n## Summary\n\nThis PR
looks to improve the performance of the newly
introduced\n`useBrowserFields` implementation. The currently
used\n`DataViewSpec.fields` isn't performant at scale and is not
cached.\nExisting code is as
follows:\n[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)\ncall
as
seen\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),\nand
finally\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).\nWhile
this may not be a significant issue at a few thousand fields, this\ndoes
not scale well as the number of fields reaches the tens to hundreds\nof
thousands.\n\n[Separate
PR](#225726) that moves\naway from
spec usage to pure `DataView` usage\n\nThe `useFieldsBrowser` hook is
improved by relying directly on the\nDataView rather than the
DataViewSpec. And there are improvements to the\noriginal sourcerer
fieldBrowser calculation. Currently the use of\n`memoizeOne` doesn't
actually work as the fields object passed to it is\nan object and not an
array.\n\nA new fieldBrowserManager was introduced that caches based on
the scope.\nOriginally investigated caching via a `WeakMap` to avoid the
scenario\nwhere all 4 (current) scopes cache the exact same DataView,
but it\nlooked like references to previously selected dataViews retained
a\nstrong reference in memory, which often led to all N number of
selected\ndataViews being retained in the cache. The current approach is
safer as\nthe number of cached entries are capped
at\n`DataViewManagerScopeName.length`\n\n\n** Relevant configurations
**\n`xpack.securitySolution.enableExperimental:\n['newDataViewPickerEnabled']`","sha":"9a18757162b038898f5dfb11f1aa3a1130ac749a"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/225859","number":225859,"mergeCommit":{"message":"[Investigations]
- Improve useFieldBrowser performance (#225859)\n\n## Summary\n\nThis PR
looks to improve the performance of the newly
introduced\n`useBrowserFields` implementation. The currently
used\n`DataViewSpec.fields` isn't performant at scale and is not
cached.\nExisting code is as
follows:\n[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)\ncall
as
seen\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),\nand
finally\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).\nWhile
this may not be a significant issue at a few thousand fields, this\ndoes
not scale well as the number of fields reaches the tens to hundreds\nof
thousands.\n\n[Separate
PR](#225726) that moves\naway from
spec usage to pure `DataView` usage\n\nThe `useFieldsBrowser` hook is
improved by relying directly on the\nDataView rather than the
DataViewSpec. And there are improvements to the\noriginal sourcerer
fieldBrowser calculation. Currently the use of\n`memoizeOne` doesn't
actually work as the fields object passed to it is\nan object and not an
array.\n\nA new fieldBrowserManager was introduced that caches based on
the scope.\nOriginally investigated caching via a `WeakMap` to avoid the
scenario\nwhere all 4 (current) scopes cache the exact same DataView,
but it\nlooked like references to previously selected dataViews retained
a\nstrong reference in memory, which often led to all N number of
selected\ndataViews being retained in the cache. The current approach is
safer as\nthe number of cached entries are capped
at\n`DataViewManagerScopeName.length`\n\n\n** Relevant configurations
**\n`xpack.securitySolution.enableExperimental:\n['newDataViewPickerEnabled']`","sha":"9a18757162b038898f5dfb11f1aa3a1130ac749a"}}]}]
BACKPORT-->
michaelolo24 added a commit that referenced this pull request Jul 10, 2025
#227004)

# Backport

This will backport the following commits from `main` to `9.1`:
- [[Investigations] - Improve useFieldBrowser performance
(#225859)](#225859)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Michael
Olorunnisola","email":"michael.olorunnisola@elastic.co"},"sourceCommit":{"committedDate":"2025-07-07T20:23:14Z","message":"[Investigations]
- Improve useFieldBrowser performance (#225859)\n\n## Summary\n\nThis PR
looks to improve the performance of the newly
introduced\n`useBrowserFields` implementation. The currently
used\n`DataViewSpec.fields` isn't performant at scale and is not
cached.\nExisting code is as
follows:\n[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)\ncall
as
seen\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),\nand
finally\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).\nWhile
this may not be a significant issue at a few thousand fields, this\ndoes
not scale well as the number of fields reaches the tens to hundreds\nof
thousands.\n\n[Separate
PR](#225726) that moves\naway from
spec usage to pure `DataView` usage\n\nThe `useFieldsBrowser` hook is
improved by relying directly on the\nDataView rather than the
DataViewSpec. And there are improvements to the\noriginal sourcerer
fieldBrowser calculation. Currently the use of\n`memoizeOne` doesn't
actually work as the fields object passed to it is\nan object and not an
array.\n\nA new fieldBrowserManager was introduced that caches based on
the scope.\nOriginally investigated caching via a `WeakMap` to avoid the
scenario\nwhere all 4 (current) scopes cache the exact same DataView,
but it\nlooked like references to previously selected dataViews retained
a\nstrong reference in memory, which often led to all N number of
selected\ndataViews being retained in the cache. The current approach is
safer as\nthe number of cached entries are capped
at\n`DataViewManagerScopeName.length`\n\n\n** Relevant configurations
**\n`xpack.securitySolution.enableExperimental:\n['newDataViewPickerEnabled']`","sha":"9a18757162b038898f5dfb11f1aa3a1130ac749a","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat
Hunting:Investigations","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[Investigations]
- Improve useFieldBrowser
performance","number":225859,"url":"https://github.com/elastic/kibana/pull/225859","mergeCommit":{"message":"[Investigations]
- Improve useFieldBrowser performance (#225859)\n\n## Summary\n\nThis PR
looks to improve the performance of the newly
introduced\n`useBrowserFields` implementation. The currently
used\n`DataViewSpec.fields` isn't performant at scale and is not
cached.\nExisting code is as
follows:\n[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)\ncall
as
seen\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),\nand
finally\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).\nWhile
this may not be a significant issue at a few thousand fields, this\ndoes
not scale well as the number of fields reaches the tens to hundreds\nof
thousands.\n\n[Separate
PR](#225726) that moves\naway from
spec usage to pure `DataView` usage\n\nThe `useFieldsBrowser` hook is
improved by relying directly on the\nDataView rather than the
DataViewSpec. And there are improvements to the\noriginal sourcerer
fieldBrowser calculation. Currently the use of\n`memoizeOne` doesn't
actually work as the fields object passed to it is\nan object and not an
array.\n\nA new fieldBrowserManager was introduced that caches based on
the scope.\nOriginally investigated caching via a `WeakMap` to avoid the
scenario\nwhere all 4 (current) scopes cache the exact same DataView,
but it\nlooked like references to previously selected dataViews retained
a\nstrong reference in memory, which often led to all N number of
selected\ndataViews being retained in the cache. The current approach is
safer as\nthe number of cached entries are capped
at\n`DataViewManagerScopeName.length`\n\n\n** Relevant configurations
**\n`xpack.securitySolution.enableExperimental:\n['newDataViewPickerEnabled']`","sha":"9a18757162b038898f5dfb11f1aa3a1130ac749a"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/225859","number":225859,"mergeCommit":{"message":"[Investigations]
- Improve useFieldBrowser performance (#225859)\n\n## Summary\n\nThis PR
looks to improve the performance of the newly
introduced\n`useBrowserFields` implementation. The currently
used\n`DataViewSpec.fields` isn't performant at scale and is not
cached.\nExisting code is as
follows:\n[DataView.toSpec](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/data_view.ts#L147)\ncall
as
seen\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/data_views/abstract_data_views.ts#L391-L422),\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/field_list.ts#L157-L159),\nand
finally\n[here](https://github.com/elastic/kibana/blob/27183690142a5590b4ad72d060c43cad869f3f3c/src/platform/plugins/shared/data_views/common/fields/data_view_field.ts#L438-L440).\nWhile
this may not be a significant issue at a few thousand fields, this\ndoes
not scale well as the number of fields reaches the tens to hundreds\nof
thousands.\n\n[Separate
PR](#225726) that moves\naway from
spec usage to pure `DataView` usage\n\nThe `useFieldsBrowser` hook is
improved by relying directly on the\nDataView rather than the
DataViewSpec. And there are improvements to the\noriginal sourcerer
fieldBrowser calculation. Currently the use of\n`memoizeOne` doesn't
actually work as the fields object passed to it is\nan object and not an
array.\n\nA new fieldBrowserManager was introduced that caches based on
the scope.\nOriginally investigated caching via a `WeakMap` to avoid the
scenario\nwhere all 4 (current) scopes cache the exact same DataView,
but it\nlooked like references to previously selected dataViews retained
a\nstrong reference in memory, which often led to all N number of
selected\ndataViews being retained in the cache. The current approach is
safer as\nthe number of cached entries are capped
at\n`DataViewManagerScopeName.length`\n\n\n** Relevant configurations
**\n`xpack.securitySolution.enableExperimental:\n['newDataViewPickerEnabled']`","sha":"9a18757162b038898f5dfb11f1aa3a1130ac749a"}}]}]
BACKPORT-->
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 10, 2025
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
## 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']`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team v8.19.0 v9.1.0 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants