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

[Security Solution] Reduce initial bundle size #78992

Merged
merged 23 commits into from
Oct 13, 2020

Conversation

oatkiller
Copy link
Contributor

@oatkiller oatkiller commented Sep 30, 2020

Summary

  • Load the redux store factory and initial state factory lazily
  • Combine certain dynamic imports to reduce total async bundle size (this works because shared dependencies between async chunks are not being de-duped by Webpack.)
  • Add explicit types in some areas.

Here is a screenshot of the webpack analyse UI with the stats.json (dist version) of our plugin as it is in master with the only change being that all dynamic imports had a webpackChunkName magic comment added:
image

Here is a screenshot of the same UI with the stats.json (dist version) or our plugin as it is in this PR.
image

Note that today the resolver chunks are only used in tests.

screenshots

The app still loads.
image

For maintainers

@oatkiller oatkiller requested review from a team as code owners September 30, 2020 16:56
@oatkiller oatkiller added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 30, 2020
import { SecurityApp } from './app';
import { AppFrontendLibs } from '../common/lib/lib';

interface RenderAppProps extends AppFrontendLibs, AppMountParameters {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this to x-pack/plugins/security_solution/public/app/types.ts. This change isn't really necessary.

@@ -27,7 +17,7 @@ export const renderApp = ({
services,
store,
SubPluginRoutes,
}: RenderAppProps) => {
}: RenderAppProps): (() => void) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding explicit types for readability.

import { ConfigureEndpointPackagePolicy } from './management/pages/policy/view/ingest_manager_integration/configure_package_policy';

import { State, createStore, createInitialState } from './common/store';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are now loaded dynamically via x-pack/plugins/security_solution/public/lazy_plugin_dependencies.tsx

import('./app'),
import('./common/lib/compose/kibana_compose'),
]);
private async downloadAssets(): Promise<{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

downloadAssets downloadSubPlugins and buildStore are all needed if and only if a user has interacted with on of the registered apps. They all pull their dynamic dependencies from x-pack/plugins/security_solution/public/lazy_plugin_dependencies.tsx now.

We could refactor the code here for clarity but I tried to keep my changes as unobtrusive as possible as the 7.10 feature freeze is nearly here.

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

@oatkiller this is fantastic, thanks for looking into this. I had one question but this looks good to me. For posterity: this should close #64589.

x-pack/plugins/security_solution/public/plugin.tsx Outdated Show resolved Hide resolved
@rylnd rylnd linked an issue Sep 30, 2020 that may be closed by this pull request
const appLibs: AppObservableLibs = { apolloClient, kibana: coreStart };
const libs$ = new BehaviorSubject(appLibs);

const detectionsStart = detectionsSubPlugin.start(this.storage);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

start is called on most of these subPlugins more than once. That seems like an issue. I'm going to look into this a bit more.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
securitySolution 2057 2058 +1

async chunks size

id before after diff
securitySolution 10.6MB 10.9MB ⚠️ +287.4KB

distributable file count

id before after diff
default 48449 48443 -6

page load bundle size

id before after diff
securitySolution 593.2KB 253.5KB -339.7KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@oatkiller oatkiller merged commit 6ab4be5 into elastic:master Oct 13, 2020
@oatkiller oatkiller deleted the reduce-bundle-size branch October 13, 2020 15:21
oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Oct 13, 2020
* Load the redux store factory and initial state factory lazily
* Combine certain dynamic imports to reduce total async bundle size (this works because shared dependencies between async chunks are not being de-duped by Webpack.)
* Add explicit types in some areas.

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Oct 13, 2020
* Load the redux store factory and initial state factory lazily
* Combine certain dynamic imports to reduce total async bundle size (this works because shared dependencies between async chunks are not being de-duped by Webpack.)
* Add explicit types in some areas.

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 13, 2020
…otphase-to-formlib

* 'master' of github.com:elastic/kibana: (59 commits)
  [Security Solution][Resolver] Replace copy-to-clipboard with native navigator.clipboard (elastic#80193)
  [Security Solution] Reduce initial bundle size (elastic#78992)
  [Security Solution][Resolver] Fix Resize node box-shadow bug (elastic#80223)
  Move observability content (elastic#79978)
  skip flaky suite (elastic#79389)
  removing kibana_datatable` in favor of `datatable` (elastic#75184)
  [ML] Fixes for anomaly swim lane  (elastic#80299)
  [Lens] Smokescreen lens test unskip (elastic#80190)
  Improved AlertsClient tests structure by splitting a huge alerts_client.tests.ts file into a specific files defined by its responsibility. (elastic#80088)
  [APM] React key warning when opening popover with external resources (elastic#80328)
  [Step 1] use Observables on server search API (elastic#79874)
  Apply back pressure in Task Manager whenever Elasticsearch responds with a 429 (elastic#75666)
  [Lens] Leverage original http request error (elastic#79831)
  [Security Solution][Case] Improve ServiceConnectorCaseParams type (elastic#80109)
  [SECURITY_SOLUTION] Fix query on alert histogram (elastic#80219)
  [DOCS] Update ingest node pipelines doc (elastic#79187)
  [Ingest Manager] Split up OpenAPI spec file  (elastic#80107)
  [SECURITY_SOLUTION][ENDPOINT] Fix label on Trusted App create name field (elastic#80001)
  [Ingest Manager] Fix agent policy bump revision to create only one POLICY_CHANGE action (elastic#80081)
  Grid layout fixes (elastic#80305)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/data_tier_allocation_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/shared_imports.ts
oatkiller pushed a commit that referenced this pull request Oct 13, 2020
* Load the redux store factory and initial state factory lazily
* Combine certain dynamic imports to reduce total async bundle size (this works because shared dependencies between async chunks are not being de-duped by Webpack.)
* Add explicit types in some areas.

Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SIEM] Plugin bundle optimizations
5 participants