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

as-clean-up-deps #1827

Merged
merged 8 commits into from
Oct 4, 2018
Merged

as-clean-up-deps #1827

merged 8 commits into from
Oct 4, 2018

Conversation

artursmirnov
Copy link
Contributor

Trying to clean up some dependencies in order to improve builds performance and reduce bundle size.

@artursmirnov
Copy link
Contributor Author

Initial build time and sizes:

File sizes:

  • dist/assets/auto-import-fastboot-e289ea484a4d8949ea84cb262a624452.js: 79 B (99 B gzipped)
  • dist/assets/travis-04bbf038795d2b6b662f0ead3613be7a.css: 289.29 KB (49.4 KB gzipped)
  • dist/assets/travis-870ffeafebb4e39e54774c2f14db93d2.js: 841.35 KB (142.71 KB gzipped)
  • dist/assets/vendor-65b4f5d5c732f6d69fabde773ab7a5f2.js: 2.6 MB (673.99 KB gzipped)
  • dist/assets/vendor-933d8ffee1afb6c839800d85d800f880.css: 6.23 KB (1.47 KB gzipped)
  • dist/check-browser-support-5112b56b4ecedda48321ba2cef8cdb49.js: 14.03 KB (5.58 KB gzipped)
  • dist/ember-fetch/fastboot-fetch-d80ae64d82d8a7b50ee10b4c17c1c4c1.js: 511 B (330 B gzipped)
  • dist/ua-parser.min-5269884f6a166a214db932a19abbb445.js: 13.83 KB (5**.5 KB** gzipped)
    ember b -prod 117.31s user 9.96s system 102% cpu 2:04.73 total

@artursmirnov
Copy link
Contributor Author

After removing ember-auto-import and related dependencies (e.g. lodash):

File sizes:

  • dist/assets/travis-04bbf038795d2b6b662f0ead3613be7a.css: 289.29 KB (49.4 KB gzipped)
  • dist/assets/travis-04e43d68e856248109c8ebcfb30837c7.js: 841.12 KB (142.65 KB gzipped)
  • dist/assets/vendor-8eb167a94142bac5804b1dda12332b6c.js: 2.58 MB (667.01 KB gzipped)
  • dist/assets/vendor-933d8ffee1afb6c839800d85d800f880.css: 6.23 KB (1.47 KB gzipped)
  • dist/check-browser-support-5112b56b4ecedda48321ba2cef8cdb49.js: 14.03 KB (5.58 KB gzipped)
  • dist/ember-fetch/fastboot-fetch-d80ae64d82d8a7b50ee10b4c17c1c4c1.js: 511 B (330 B gzipped)
  • dist/ua-parser.min-5269884f6a166a214db932a19abbb445.js: 13.83 KB (5.5 KB gzipped)
    ember b -prod 62.78s user 6.57s system 115% cpu 1:00.19 total

@@ -22,15 +22,14 @@
"@ember/jquery": "^0.5.2",
"@ember/optional-features": "^0.6.3",
"active-model-adapter": "2.2.0",
"ansiparse": "0.1.0",
"ansiparse": "artursmirnov/ansiparse",

This comment was marked as spam.

@artursmirnov
Copy link
Contributor Author

I'm not done yet, there will be some other optimizations, although, if any of you don't like it, better to shout it earlier ;)

Copy link
Contributor

@backspace backspace left a comment

Choose a reason for hiding this comment

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

wow, I had no idea ember-auto-import would slow things down so much. I’ve periodically struggled with imports, the various formats aren’t something I know much about. Thanks for doing this!

@artursmirnov
Copy link
Contributor Author

So now it's ready. The final result:

File sizes:

  • dist/assets/travis-04bbf038795d2b6b662f0ead3613be7a.css: 289.29 KB (49.4 KB gzipped)
  • dist/assets/travis-aa3952e127c49097e99076134efc1d66.js: 833.5 KB (142.13 KB gzipped)
  • dist/assets/vendor-933d8ffee1afb6c839800d85d800f880.css: 6.23 KB (1.47 KB gzipped)
  • dist/assets/vendor-998be8890a46314df39e4e6e49efc0ef.js: 2.54 MB (661.16 KB gzipped)
  • dist/check-browser-support-5112b56b4ecedda48321ba2cef8cdb49.js: 14.03 KB (5.58 KB gzipped)
  • dist/ember-fetch/fastboot-fetch-d80ae64d82d8a7b50ee10b4c17c1c4c1.js: 511 B (330 B gzipped)
  • dist/ua-parser.min-5269884f6a166a214db932a19abbb445.js: 13.83 KB (5.5 KB gzipped)
    ember b -prod 74.61s user 7.47s system 114% cpu 1:00.78 total

@artursmirnov artursmirnov merged commit 0e81278 into master Oct 4, 2018
@artursmirnov artursmirnov deleted the as-clean-up-deps branch October 4, 2018 09:28
@ef4
Copy link
Contributor

ef4 commented Feb 4, 2019

I was curious about this change, so I tested the size impact of using ember-auto-import vs your hand-rolled list of importNpmDependency calls. The impact is zero. ember-auto-import produces exactly the same size production build as you get doing it manually.

All the size benefit from the PR comes from removing heavy dependencies like lodash. You would get the same benefit if you kept ember-auto-import and just stopped importing from lodash, etc.

@artursmirnov
Copy link
Contributor Author

@ef4 thank you for your feedback, it's highly appreciated! The main purpose of this PR was to improve build time first of all, because we have some internal tools for deployments automation, which were failing due to timeouts. For some reason, ember-auto-imports doubled the build time for travis-web. Maybe it was also caused by some massive imports. It's definitely worth trying to add the add-on to the current state and see how it works.

Although, I really like the add-on and would love to get it back to travis-web if only it didn't make such a significant impact to build times. If you have any ideas regarding how to improve that, we would appreciate your guidance!

@ef4
Copy link
Contributor

ef4 commented Feb 5, 2019

Ah, that's interesting. Back when I first added it, I benchmarked first and it improved initial build time by about 10% and rebuild time by 30%. I will need to dig in and see if that has changed.

@ef4
Copy link
Contributor

ef4 commented Feb 5, 2019

Oh, so one reason for those speedups was I was replacing ember-browserify. So that was not necessarily the relevant comparison for this discussion, since that is now gone as well.

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

Successfully merging this pull request may close these issues.

3 participants