Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid async imports that execute during plugin setup and start methods #194171

Open
nreese opened this issue Sep 26, 2024 · 4 comments
Open

avoid async imports that execute during plugin setup and start methods #194171

nreese opened this issue Sep 26, 2024 · 4 comments
Assignees
Labels
Meta :ml Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@nreese
Copy link
Contributor

nreese commented Sep 26, 2024

Problematic pattern

Avoid the pattern below, where plugin setup or start method async imports code during method execution.

Promise.all([
  asyncCheck,
  import('./fileOne'),
  import('./fileTwo'),
]).then(
  ([
    asyncCheckResults,
    { registerStuffOne },
    { registerStuffTwo },
  ]) => {
    if (asyncCheckResults) {
      registerStuffOne();
      registerStuffTwo();
    }
  }
);

This results in poor performance because the first Kibana page load requires additional server requests to download async modules. In the screen shot below, notice how there are 5 additional requests made to download dashboard bundles.
Image

This pattern masks the true page load bundle size, as async imported code is not counted towards page load bundle size.

Examples that follow this anti-pattern

  1. src/plugins/dashboard/public/plugin.tsx
  2. x-pack/plugins/aiops/public/plugin.tsx

Preferred pattern

Instead, synchronously import code in the plugin so that true page load bundle size is captured.

import { registerStuffOne } from './fileOne';
import { registerStuffTwo } from './fileTwo';
Promise.all([
  asyncCheck,
]).then(
  (asyncCheckResults) => {
    if (asyncCheckResults) {
      registerStuffOne();
      registerStuffTwo();
    }
  }
);
@nreese nreese added the Meta label Sep 26, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 26, 2024
@thomasneirynck
Copy link
Contributor

ML has an example of this as well:
#189755

@marius-dr marius-dr added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 8, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Oct 8, 2024
nreese added a commit that referenced this issue Oct 9, 2024
…5616)

Part of #194171

Notice in the screen shot below, how opening Kibana home page loads
async dashboard chunks.
<img width="500" alt="Screenshot 2024-10-09 at 8 38 24 AM"
src="https://github.com/user-attachments/assets/2cfdb512-03e4-4634-bb0c-a8d163f98540">

This PR replaces async import with a sync import. The page load bundle
size increases but this is no different than the current behavior, just
the import is now properly accounted for in page load stats. The next
step is to resolve #191642 and
reduce the page load impact of registering uiActions (but this is out of
scope for this PR).
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 9, 2024
…stic#195616)

Part of elastic#194171

Notice in the screen shot below, how opening Kibana home page loads
async dashboard chunks.
<img width="500" alt="Screenshot 2024-10-09 at 8 38 24 AM"
src="https://github.com/user-attachments/assets/2cfdb512-03e4-4634-bb0c-a8d163f98540">

This PR replaces async import with a sync import. The page load bundle
size increases but this is no different than the current behavior, just
the import is now properly accounted for in page load stats. The next
step is to resolve elastic#191642 and
reduce the page load impact of registering uiActions (but this is out of
scope for this PR).

(cherry picked from commit 7448376)
walterra added a commit that referenced this issue Nov 8, 2024
## Summary

Part of #194171.

The AIOps plugin loads quite some bundle chunks on the first Kibana
load. On main it currently looks like this on page load:

![CleanShot 2024-11-04 at 14 56
52@2x](https://github.com/user-attachments/assets/cbabeaa2-848c-4970-aea5-b06befed0bec)

This means while the `aiops.plugin.js` bundle has just 10KB, we load
290KB in total via async bundles on every Kibana full page load.

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load. This increases
`aiops.plugin.js` to ~15KB, but gets rid of all the rest!

![CleanShot 2024-11-04 at 15 02
10@2x](https://github.com/user-attachments/assets/557e240f-2c1c-434d-a936-dddb11ab68b6)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Nov 8, 2024
## Summary

Part of elastic#194171.

The AIOps plugin loads quite some bundle chunks on the first Kibana
load. On main it currently looks like this on page load:

![CleanShot 2024-11-04 at 14 56
52@2x](https://github.com/user-attachments/assets/cbabeaa2-848c-4970-aea5-b06befed0bec)

This means while the `aiops.plugin.js` bundle has just 10KB, we load
290KB in total via async bundles on every Kibana full page load.

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load. This increases
`aiops.plugin.js` to ~15KB, but gets rid of all the rest!

![CleanShot 2024-11-04 at 15 02
10@2x](https://github.com/user-attachments/assets/557e240f-2c1c-434d-a936-dddb11ab68b6)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)

(cherry picked from commit 33c303d)
rbrtj pushed a commit to rbrtj/kibana that referenced this issue Nov 8, 2024
Part of elastic#194171.

The AIOps plugin loads quite some bundle chunks on the first Kibana
load. On main it currently looks like this on page load:

![CleanShot 2024-11-04 at 14 56
52@2x](https://github.com/user-attachments/assets/cbabeaa2-848c-4970-aea5-b06befed0bec)

This means while the `aiops.plugin.js` bundle has just 10KB, we load
290KB in total via async bundles on every Kibana full page load.

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load. This increases
`aiops.plugin.js` to ~15KB, but gets rid of all the rest!

![CleanShot 2024-11-04 at 15 02
10@2x](https://github.com/user-attachments/assets/557e240f-2c1c-434d-a936-dddb11ab68b6)

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
walterra added a commit that referenced this issue Nov 11, 2024
## Summary

Part of #194171.

The `dataVisualizer` plugin loads quite some bundle chunks on the first
Kibana load. On `main` it currently looks like this on page load:

![CleanShot 2024-11-05 at 17 51
35@2x](https://github.com/user-attachments/assets/7b2fcc7e-08fb-48b8-a1d9-03063b555e38)

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load.

![CleanShot 2024-11-06 at 10 51
36@2x](https://github.com/user-attachments/assets/019c3c79-49b5-4e63-9b94-3d7b02963a9b)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Nov 11, 2024
## Summary

Part of elastic#194171.

The `dataVisualizer` plugin loads quite some bundle chunks on the first
Kibana load. On `main` it currently looks like this on page load:

![CleanShot 2024-11-05 at 17 51
35@2x](https://github.com/user-attachments/assets/7b2fcc7e-08fb-48b8-a1d9-03063b555e38)

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load.

![CleanShot 2024-11-06 at 10 51
36@2x](https://github.com/user-attachments/assets/019c3c79-49b5-4e63-9b94-3d7b02963a9b)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)

(cherry picked from commit aa4f430)
tkajtoch pushed a commit to tkajtoch/kibana that referenced this issue Nov 12, 2024
## Summary

Part of elastic#194171.

The `dataVisualizer` plugin loads quite some bundle chunks on the first
Kibana load. On `main` it currently looks like this on page load:

![CleanShot 2024-11-05 at 17 51
35@2x](https://github.com/user-attachments/assets/7b2fcc7e-08fb-48b8-a1d9-03063b555e38)

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load.

![CleanShot 2024-11-06 at 10 51
36@2x](https://github.com/user-attachments/assets/019c3c79-49b5-4e63-9b94-3d7b02963a9b)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
@walterra
Copy link
Contributor

walterra commented Nov 13, 2024

Thanks for documenting this. We fixed it now for the aiops and dataVisualizer plugin:

Doing it for the ml plugin is more work, but we have a PoC already that shows that we can fix the pattern and keep the loading bundle below 100Kbytes. Like mentioned elsewhere, it would be great if UI actions could be registered asynchronously!

@walterra walterra self-assigned this Nov 13, 2024
@walterra walterra added the :ml label Nov 13, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Nov 18, 2024
## Summary

Part of elastic#194171.

The `dataVisualizer` plugin loads quite some bundle chunks on the first
Kibana load. On `main` it currently looks like this on page load:

![CleanShot 2024-11-05 at 17 51
35@2x](https://github.com/user-attachments/assets/7b2fcc7e-08fb-48b8-a1d9-03063b555e38)

This PR refactors how embeddables and UI actions get initialized to
avoid loading any additional async bundles on page load.

![CleanShot 2024-11-06 at 10 51
36@2x](https://github.com/user-attachments/assets/019c3c79-49b5-4e63-9b94-3d7b02963a9b)

### Checklist

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta :ml Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

5 participants