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

[ML] Fix initial plugin's bundle size #74047

Merged
merged 10 commits into from
Aug 5, 2020

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Aug 1, 2020

Summary

Reduces ML plugin initial bundle size. There is still room for improvement, e.g. we can change lodash and semver imports, some utils functions, but this PR already makes it significantly better ( 4.3MB -> 499KB).

image

Checklist

@darnautov darnautov self-assigned this Aug 1, 2020
@darnautov darnautov marked this pull request as ready for review August 4, 2020 06:30
@darnautov darnautov requested a review from a team as a code owner August 4, 2020 06:30
@darnautov darnautov added :ml release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Aug 4, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@darnautov darnautov requested a review from a team as a code owner August 4, 2020 08:23
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

Added a couple of questions.
Tested and LGTM

x-pack/plugins/ml/public/embeddables/index.ts Outdated Show resolved Hide resolved
x-pack/plugins/ml/public/plugin.ts Outdated Show resolved Hide resolved
@darnautov darnautov requested a review from walterra August 5, 2020 09:27
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Code LGTM.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
ml 1113 -115 1228

async chunks size

id value diff baseline
ml 7.9MB +7.9MB 15.3KB

page load bundle size

id value diff baseline
ml 500.3KB -3.8MB 4.3MB

History

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

@darnautov darnautov merged commit 8231b0c into elastic:master Aug 5, 2020
@darnautov darnautov deleted the ML-split-bundle branch August 5, 2020 11:18
darnautov added a commit to darnautov/kibana that referenced this pull request Aug 5, 2020
* [ML] use dynamic imports

* [ML] fix react-use imports

* [ML] change embeddables imports

* [ML] embeddable exports

* [ML] move SCSS import

* [ML] management page styles

* [ML] refactor with types and constants files

* [ML] move declarations
darnautov added a commit to darnautov/kibana that referenced this pull request Aug 5, 2020
* [ML] use dynamic imports

* [ML] fix react-use imports

* [ML] change embeddables imports

* [ML] embeddable exports

* [ML] move SCSS import

* [ML] management page styles

* [ML] refactor with types and constants files

* [ML] move declarations
Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

Change requested on switching the import in the jobs_list

Comment on lines +2 to +4
#kibanaManagementMLSection {
@import 'components/index';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we switch this so it uses a three-letter prefix on the classes, instead of nesting within an id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mdefazio! Sorry, I already merged this one, but the point of this PR was to rearrange some imports to reduce the bundle size we deliver during kibana loading. We definitely need to revisit our SCSS files, I believe it deserves a dedicated PR.

darnautov added a commit that referenced this pull request Aug 5, 2020
* [ML] use dynamic imports

* [ML] fix react-use imports

* [ML] change embeddables imports

* [ML] embeddable exports

* [ML] move SCSS import

* [ML] management page styles

* [ML] refactor with types and constants files

* [ML] move declarations
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 5, 2020
* master: (154 commits)
  [ML] Fix initial plugin's bundle size (elastic#74047)
  [Ingest Manager] prevent crash on unhandled rejection from setupIngestManager (elastic#74300)
  [Logs UI] Correct trial period duration in anomaly splash screen (elastic#74249)
  [Discover] Inline noWhiteSpace function (elastic#74331)
  [DOCS] Add Observability topic (elastic#73041)
  skip flaky suite (elastic#74327)
  [Security Solution][Detections] Fixes Severity Override not matching for Elastic Endpoint Security rule (elastic#74317)
  [Security Solution][Exceptions] - Fixes exceptions builder nested deletion issue and adds unit tests (elastic#74250)
  Fixed Alert details does not update page title and breadcrumb (elastic#74214)
  [src/dev/build] build Kibana Platform bundles from source (elastic#73591)
  [Reporting] Shorten asset path to help CLI FS Watcher (elastic#74185)
  Fix TMS not loaded in legacy maps (elastic#73570)
  [Security Solution] styling for notes' panel (elastic#74274)
  [Security Solution][Tech Debt] cleans up ts-ignore issues and some smaller linter issues  (elastic#74268)
  Make the actions plugin support generics (elastic#71439)
  [Security Solution] Keep original note creator (elastic#74203)
  [CI] Fix xpack kibana build dir in xpack visual regression script
  [CI] Fix baseline_capture job by adding parallel process number back
  [Monitoring] Ensure setup mode works on cloud but only for alerts (elastic#73127)
  [Maps] Custom color ramps should show correctly on the map for mvt layers (elastic#74169)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants