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

[optimizer] More aggressive chunking of common/vendor code #15816

Merged
merged 3 commits into from
Jan 5, 2018

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Jan 2, 2018

Previously, we were not aggressive in combining common code which resulted in duplicates included in the bundles.

As an example node_modules/elasticsearch-browser/elasticsearch.angular.js is present in the following chunks:

  • kibana.bundle.js
  • dashboardViewer.bundle.js
  • apm.bundle.js
  • monitoring.bundle.js
  • ml.bundle.js
  • timelion.bundle.js
  • graph.bundle.js

Vendor code (anything inside Kibana's node_modules) is placed in vendors.bundle.js while everything else with more than two references is placed in commons.bundle.js.

This has a couple positive side-effects (numbers are with x-pack & canvas):

  • Decreased build time. Seeing builds go from 475.76 seconds to 274.72.
  • Decreased memory overhead. Uses roughly 1/3 the memory overhead.
  • Decreased bundle size. A 68% reduction in overall bundle size. Going from 66.16 MB to 21.13 MB.
master vender-assets
apm.bundle.js 7,834,652 1,708,174
apm.style.css 362,146 4,163
canvas.bundle.js 8,043,749 319,317
canvas.style.css 646,826 161,368
commons.bundle.js 4,217,220 8,213,974
commons.style.css 29,327 515,278
dashboardViewer.bundle.js 8,229,866 2,742
dashboardViewer.style.css 508,630
graph.bundle.js 4,500,931 133,069
graph.style.css 358,401 5,045
kibana.bundle.js 10,306,521 1,186,511
kibana.style.css 602,698 70,842
login.bundle.js 298,389 5,076
login.style.css 341,004 2,206
logout.bundle.js 1,378 1,355
logout.style.css 77 77
ml.bundle.js 5,310,904 833,213
ml.style.css 488,853 112,434
monitoring.bundle.js 6,527,345 438,853
monitoring.style.css 373,759 15,776
sense-tests.bundle.js 660,960 93,293
sense-tests.style.css 4,684 4,684
stateSessionStorageRedirect.bundle.js 294,762 1,450
stateSessionStorageRedirect.style.css 338,798
status_page.bundle.js 311,252 6,282
status_page.style.css 374,716 35,918
timelion.bundle.js 4,783,638 54,475
timelion.style.css 376,614 5,473
vendor.bundle.js 7,110,833
vendor.style.css 51,433
Total: 66,128,100 21,094,305

screenshot 2018-01-02 10 11 57

Release Note: Improved the build optimize time by more aggressively chunking common code, resulting in the removal of duplicate code. This drastically cuts the build and plugin install time and overall bundle asset size.

}),

new NoEmitOnErrorsPlugin(),
new webpack.optimize.CommonsChunkPlugin({
name: 'vendor',
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably be vendors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, missed it when I changed from vendor to vendors.

Previously, we were not aggressive in combining common code which resulted in duplicates included in the bundles.

As an example `node_modules/elasticsearch-browser/elasticsearch.angular.js` is present in the following chunks:

* kibana.bundle.js
* dashboardViewer.bundle.js
* apm.bundle.js
* monitoring.bundle.js
* ml.bundle.js
* timelion.bundle.js
* graph.bundle.js

Vendored code (anything inside node_modules) is placed in vendors.bundle.js while everything else with more than two references is placed in commons.bundle.js.

This has a couple positive side-effects (numbers are with x-pack & canvas):

* Decreased build time. Seeing builds go from 475.76 seconds to 274.72.
* Decreased memory overhead. Uses roughly 1/3 the memory overhead.
* Decreased bundle size. A 68% reduction in overall bundle size. Going from 66.16 MB to 21.13 MB.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@kimjoar
Copy link
Contributor

kimjoar commented Jan 3, 2018

This is looking great!

Do you know if it's simple/possible to split up the vendors file? I’m not too versed in parse times, but I think ~11MB might take in the range of 2-4 seconds to parse in Chrome and Firefox on a brand new Macbook Pro. This is from the latest Kibana release when loading the current 10MB kibana.bundle.js:

screen shot 2018-01-03 at 14 45 17

(Given we're already on 10MB today, moving to 11MB likely isn't a problem in the short-term as this change is giving us a couple essential wins, but it's probably something we want to explore more later)

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

The code change looks good to me and it seems to improve the dev experience 💯

@tylersmalley tylersmalley changed the title [optimize] More aggressive chunking of common/vendor code [optimizer] More aggressive chunking of common/vendor code Jan 4, 2018
@rhoboat
Copy link

rhoboat commented Jan 4, 2018

@tylersmalley help me understand how

while everything else with more than two references is placed in commons.bundle.js.

improves things?

@tylersmalley
Copy link
Contributor Author

tylersmalley commented Jan 4, 2018

@archanid, if there is code required in Kibana and X-Pack that code will be placed in commons instead of duplicated in both entry points.

@w33ble
Copy link
Contributor

w33ble commented Jan 4, 2018

With no custom optimize rules:

Seems to work with both x-pack and canvas running. I'm not running into the memory problems like before, and build time is around 260 seconds. A couple of notes:

  • I'm seeing separate .js.map files in the bundle path, but was told sourcemaps should be inline
  • Source maps in Canvas don't seem to work in Chrome, everything references the common bundle

With custom optimize rules:

Using the following in my kibana.yml, I hit the OOM issue without the memory flag (crashed at ~1.4gb)

optimize:
  lazy: true
  lazyPrebuild: false
  unsafeCache: true
  profile: true
  sourceMaps: "#cheap-source-map"

@w33ble
Copy link
Contributor

w33ble commented Jan 4, 2018

Sourcemaps work fine, I loaded another plugin from the template, and even running that, the canvas extras, canvas, and x-pack, everything seems to be working correctly. And I'm not hitting OOM issues, even when using `#cheap-module-eval-source-map`. I still see all those `.map` files, but apparently that's expected after all. 

No idea what changed, but it's not the first time sourcemaps have been flaky for me...

Built x-pack, canvas, canvas-extras, and a generated plugin in 161 seconds, with the following in my kibana.dev.yml, without issue, and the sourcemaps work great.

**kibana.dev.yml**

```yml
optimize:
  lazy: true
  lazyPrebuild: false
  sourceMaps: "#cheap-module-eval-source-map"
```

@epixa
Copy link
Contributor

epixa commented Jan 5, 2018

LGTM

@w33ble
Copy link
Contributor

w33ble commented Jan 5, 2018

Tried doing a "production" install on my machine with 6.1.2 snapshots.

  • x-pack: real 5m57.054s
  • canvas (with x-pack installed): real 6m51.570s
  • canvas-extras (with x-pack and canvas installed: real 6m31.852s

Never ran into any OOM issues either.

There's a problem running canvas, but I'm pretty sure it's not a new problem. Otherwise, it looks like everything else is working as expected. I can't speak to the webpack changes, I'm not well versed enough, but functionally, this PR LGTM.

@tylersmalley tylersmalley merged commit 92b373b into elastic:master Jan 5, 2018
tylersmalley added a commit that referenced this pull request Jan 5, 2018
Previously, we were not aggressive in combining common code which resulted in duplicates included in the bundles.

As an example `node_modules/elasticsearch-browser/elasticsearch.angular.js` is present in the following chunks:

* kibana.bundle.js
* dashboardViewer.bundle.js
* apm.bundle.js
* monitoring.bundle.js
* ml.bundle.js
* timelion.bundle.js
* graph.bundle.js

Vendor code (anything inside Kibana's node_modules) is placed in vendors.bundle.js while everything else with more than two references is placed in commons.bundle.js.

This has a couple positive side-effects (numbers are with x-pack & canvas):

* Decreased build time. Seeing builds go from 475.76 seconds to 274.72.
* Decreased memory overhead. Uses roughly 1/3 the memory overhead.
* Decreased bundle size. A 68% reduction in overall bundle size. Going from 66.16 MB to 21.13 MB.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley
Copy link
Contributor Author

6.2/6.x: 303f98a

@epixa
Copy link
Contributor

epixa commented Jan 6, 2018

@tylersmalley Since this has impacts on production builds, can you label accordingly so it appears in release notes?

@epixa
Copy link
Contributor

epixa commented Jan 7, 2018

This ended up breaking x-pack ci, so it has been reverted until we can figure out what changed for plugins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants