[unified search] optimize async chunk loading by nreese · Pull Request #214483 · elastic/kibana

nreese · 2025-03-13T20:29:28Z

PR reduces unifiedSearch chunks into ui chunk, action chunk, and a autocomplete chunk.

Before

After

The second chunk request is because search bar loads KQL suggestions. This will be addressed in a follow up PR and the search bar will lazy load suggestions only when interacted with.

nreese · 2025-03-13T20:29:36Z

/ci

…-fix'

nreese · 2025-03-13T20:57:02Z

/ci

nreese · 2025-03-14T14:50:25Z

/ci

nreese · 2025-03-14T15:16:42Z

/ci

nreese · 2025-03-14T15:17:46Z

x-pack/solutions/security/plugins/security_solution/public/plugin.tsx

      order: 1,
      mount: async (params) => {
-        const { renderApp, services, store } = await mountDependencies();
-        const { ManagementSettings } = await this.lazyAssistantSettingsManagement();


Not part of fix but I saw this while in the file. This creates a waterfall of chunk loading. I moved ManagementSettings chunk loading into mountDependencies to avoid this waterfall.

x-pack/solutions/security/plugins/security_solution/public/plugin.tsx

nreese · 2025-03-14T15:22:41Z

/ci

nreese · 2025-03-14T15:51:21Z

@elasticmachine merge upstream

nreese · 2025-03-14T15:55:20Z

/ci

nreese · 2025-03-14T18:15:21Z

/ci

…-fix'

semd · 2025-03-24T16:58:30Z

x-pack/solutions/security/plugins/security_solution/public/plugin.tsx

-        await this.registerActions(store, params.history, core, services);
+        if (!this._actionsRegistered) {
+          this._actionsRegistered = true;
+          registerActions(store, params.history, core, services);


Isn't this function async now? Don't you think we should await?

The results of registerActions are not used so I did not think an await was needed. I will defer to your team if await is required. Just let me know.

This function is async now because you changed that here

Does this really need to be async?

Does this really need to be async?

Yes, because createFilterAction from import { createFilterAction } from '@kbn/unified-search-plugin/public'; is now async to avoid including createFilterAction in unifiedSearch bundle

Added await e1bba66

Okay, but this is not awaiting anything unless we also await the registerDiscoverHistogramActions call inside registerUIActions, right?

registerUIActions needs to be async so that registerDiscoverHistogramActions can be async. Let me know if you have any further questions

@nreese Sure, let me rephrase the question.

Why is the registerDiscoverHistogramActions() call not awaited here?

kibana/x-pack/solutions/security/plugins/security_solution/public/app/actions/register.ts

Line 56 in e1bba66

registerDiscoverHistogramActions(store, history, coreSetup, services);

Thanks for clarifying, added await 98fc7a7

semd

LGTM

nreese · 2025-03-26T14:28:00Z

@elasticmachine merge upstream

…-fix'

nreese · 2025-03-26T17:15:40Z

@elasticmachine merge upstream

ThomThomson

Tested this locally in chrome and looked through the code. Nice clean change list, great bundle reductions, and a good start to our ownership of Unified Search.

Changes LGTM! Left a few small comments.

ThomThomson · 2025-03-26T16:16:11Z

packages/kbn-optimizer/limits.yml

  unifiedDocViewer: 25099
  unifiedHistogram: 19928
-  unifiedSearch: 71059
+  unifiedSearch: 23000


Great to see this coming down!

ThomThomson · 2025-03-26T16:40:47Z

...ns/shared/unified_search/public/actions/apply_filter_action/apply_filter_popover_content.tsx

@@ -42,9 +42,7 @@ interface State {
  fieldLabel?: string;
 }

-// Needed for React.lazy
-// eslint-disable-next-line import/no-default-export
-export default class ApplyFiltersPopoverContent extends Component<Props, State> {


Good to see this loaded async with the action definition rather than via React lazy.

ThomThomson · 2025-03-26T16:42:13Z

src/platform/plugins/shared/unified_search/public/dataview_picker/index.tsx

 * the withSuspense` HOC to load this component.
 */
-export const DataViewsListLazy = React.lazy(() => import('./dataview_list'));
+export const DataViewsListLazy = React.lazy(async () => {


Eventually it would be good to have these exported with their own load states rather than forcing the consumer to use suspense and provide a fallback.

ThomThomson · 2025-03-26T17:50:06Z

src/platform/plugins/shared/unified_search/public/filter_badge/filter_badge_group.tsx

-
-// Needed for React.lazy
-// eslint-disable-next-line import/no-default-export
-export default FilterBadgeGroup;


Great to see the return { default: Component } syntax used for the lazy loading rather than disabling our no-default-export rule.

ThomThomson · 2025-03-26T17:55:46Z

src/platform/plugins/shared/unified_search/public/index.ts

-export { QuerySuggestionTypes } from './autocomplete/providers/query_suggestion_provider';
-
-import { UnifiedSearchPublicPlugin } from './plugin';
+export async function createFilterAction(


I see this is used specifically in Discover. Seems like a strange use case to import an action definition from another plugin and register it again. It's probably out of scope for this PR to separate it out or fix this up but it's definitely a bit of a code smell.

For instance, why can't the trigger just get the actions from a different trigger or get the apply filter action from the registry?

nreese · 2025-03-26T20:12:26Z

@elasticmachine merge upstream

elasticmachine · 2025-03-26T23:19:36Z

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #72 / InfraOps App Metrics UI Infrastructure Source Configuration with metrics present renders the waffle map
[job] [logs] FTR Configs #97 / Security Solution - Telemetry Security Telemetry - Indices metadata task telemetry @ess @serverless indices metadata should publish data stream events

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
`securitySolution`	8.9MB	8.9MB	+60.0B
`unifiedSearch`	350.8KB	351.2KB	+483.0B
total			+543.0B

Page load bundle

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

id	before	after	diff
`securitySolution`	88.5KB	88.5KB	+6.0B
`unifiedSearch`	36.6KB	22.0KB	-14.6KB
total			-14.6KB

Unknown metric groups

async chunk count

id	before	after	diff
`unifiedSearch`	21	4	-17

ESLint disabled line counts

id	before	after	diff
`unifiedSearch`	23	8	-15

References to deprecated APIs

id	before	after	diff
`unifiedSearch`	15	13	-2

Total ESLint disabled count

id	before	after	diff
`unifiedSearch`	23	8	-15

History

💔 Build #287449 failed aff098f
💔 Build #287362 failed e338ca8
💚 Build #287021 succeeded 98fc7a7
💚 Build #286760 succeeded bb85ccb
💔 Build #286742 failed 464e433

kibanamachine · 2025-03-27T00:40:14Z

Starting backport for target branches: 8.x

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

PR reduces unifiedSearch chunks into ui chunk, action chunk, and a autocomplete chunk. ### Before <img width="350" alt="Screenshot 2025-03-14 at 8 47 10 AM" src="https://github.com/user-attachments/assets/f54fe21e-7548-48a1-8874-e36377826701" /> ### After The second chunk request is because search bar loads KQL suggestions. This will be addressed in a follow up PR and the search bar will lazy load suggestions only when interacted with. <img width="350" alt="Screenshot 2025-03-14 at 12 56 28 PM" src="https://github.com/user-attachments/assets/8f23ee56-a57a-489b-aeab-caa30f739d03" /> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 3bc1465)

kibanamachine · 2025-03-27T00:45:58Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2025-03-31T00:48:37Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

# Backport This will backport the following commits from `main` to `8.x`: - [[unified search] optimize async chunk loading (#214483)](#214483)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Nathan Reese <reese.nathan@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

PR reduces unifiedSearch chunks into ui chunk, action chunk, and a autocomplete chunk. ### Before <img width="350" alt="Screenshot 2025-03-14 at 8 47 10 AM" src="https://github.com/user-attachments/assets/f54fe21e-7548-48a1-8874-e36377826701" /> ### After The second chunk request is because search bar loads KQL suggestions. This will be addressed in a follow up PR and the search bar will lazy load suggestions only when interacted with. <img width="350" alt="Screenshot 2025-03-14 at 12 56 28 PM" src="https://github.com/user-attachments/assets/8f23ee56-a57a-489b-aeab-caa30f739d03" /> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

nreese added 2 commits March 13, 2025 14:18

single chunk

827bd17

move scss into chunk

872d9eb

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

2a5f117

…-fix'

nreese added 4 commits March 14, 2025 07:26

fix jest test

929fbe1

move actions into module

67cae59

remove unnessary comments

818ae3f

limits

37a1365

tslint security solution

5d64a90

nreese commented Mar 14, 2025

View reviewed changes

x-pack/solutions/security/plugins/security_solution/public/plugin.tsx Show resolved Hide resolved

limits.yml

e3b3065

nreese changed the title ~~[unified search] load unified search in single chunk~~ [unified search] optimize async chunk loading Mar 14, 2025

elasticmachine and others added 2 commits March 14, 2025 15:51

Merge branch 'main' into unified_search_single_chunk

14eb1d8

tslint

e835416

put autocomplete back into its own chunk

93a69b0

nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Mar 14, 2025

nreese marked this pull request as ready for review March 14, 2025 21:00

nreese requested a review from a team as a code owner March 14, 2025 21:00

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

bc3bca7

…-fix'

jbudz approved these changes Mar 14, 2025

View reviewed changes

merge with main

c98336c

semd reviewed Mar 24, 2025

View reviewed changes

nreese added 4 commits March 24, 2025 11:33

revert changes to security plugin loading

464e433

eslint

bb85ccb

add await

e1bba66

add await to registerDiscoverHistogramActions call

98fc7a7

semd approved these changes Mar 25, 2025

View reviewed changes

elasticmachine and others added 2 commits March 26, 2025 16:28

Merge branch 'main' into unified_search_single_chunk

c3a3489

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

e338ca8

…-fix'

elastic-vault-github-plugin-prod bot requested a review from a team as a code owner March 26, 2025 14:51

Merge branch 'main' into unified_search_single_chunk

aff098f

ThomThomson approved these changes Mar 26, 2025

View reviewed changes

Merge branch 'main' into unified_search_single_chunk

3968c45

nreese merged commit 3bc1465 into elastic:main Mar 27, 2025
10 checks passed

kibanamachine mentioned this pull request Mar 27, 2025

[8.x] [unified search] optimize async chunk loading (#214483) #216115

Merged

kibanamachine mentioned this pull request Mar 27, 2025

Audit info and error logs in observability plugin #216111

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 31, 2025

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 31, 2025

Conversation

nreese commented Mar 13, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before

After

Uh oh!

nreese commented Mar 13, 2025

Uh oh!

nreese commented Mar 13, 2025

Uh oh!

nreese commented Mar 14, 2025

Uh oh!

nreese commented Mar 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nreese commented Mar 14, 2025

Uh oh!

nreese commented Mar 14, 2025

Uh oh!

nreese commented Mar 14, 2025

Uh oh!

nreese commented Mar 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

semd left a comment

Choose a reason for hiding this comment

Uh oh!

nreese commented Mar 26, 2025

Uh oh!

nreese commented Mar 26, 2025

Uh oh!

ThomThomson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nreese commented Mar 26, 2025

Uh oh!

elasticmachine commented Mar 26, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

Page load bundle

async chunk count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

Uh oh!

Uh oh!

kibanamachine commented Mar 27, 2025

nreese commented Mar 13, 2025 •

edited by kibanamachine

Loading