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

Selective lazy compilation #9166

Merged
merged 9 commits into from
Aug 25, 2023
Merged

Conversation

marcins
Copy link
Contributor

@marcins marcins commented Jul 31, 2023

↪️ Pull Request

This PR adds a feature to lazy compilation to allow specifying which file paths should be (or should not be) compiled lazily.
The use case is, in a large code base you might have several levels of async imports used for lazy rendering of functionality, as well as for routing. With lazy compilation enabled this can cause a waterfall of lazy requests that can affect the usability of the feature, as several round-trips may be required for the compilation to "settle" - and as the visible dependency graph grows, so do the build times between each step.

While at least for our use case only --lazy-include is required, --lazy-exclude has also been implemented should a user desire to have a particular part of the dependency graph never be lazy compiled irrespective of other options.

💻 Examples

With selective lazy compilation you can, for example, choose to only have the lazy boundary at the route level. This means your "shell" and router always compile, then you request just the route bundle that you're currently on, and all child async bundles are included in this compilation (e.g. parcel serve --lazy --lazy-include "src/routes/**").

🚨 Test instructions

Integration tests for the new functionality have been added.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just some small fixes here that helped me debug this feature, as some attributes were missing from graphviz.

@parcel-benchmark
Copy link

parcel-benchmark commented Jul 31, 2023

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.55s -36.00ms
Cached 307.00ms +28.00ms ⚠️

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 242.00ms -16.00ms 🚀
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 250.00ms -15.00ms 🚀
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 251.00ms -15.00ms 🚀
dist/legacy/index.8692583b.js 1.48kb +0.00b 353.00ms -18.00ms 🚀
dist/legacy/index.a2819fc3.js 1.06kb +0.00b 353.00ms -18.00ms 🚀
dist/modern/index.d90ef1a6.js 917.00b +0.00b 353.00ms -19.00ms 🚀
dist/legacy/index.html 826.00b +0.00b 392.00ms -20.00ms 🚀
dist/modern/index.html 749.00b +0.00b 391.00ms -20.00ms 🚀
dist/legacy/index.b8ae99ba.css 94.00b +0.00b 252.00ms -14.00ms 🚀
dist/modern/index.31cedca9.css 94.00b +0.00b 251.00ms -15.00ms 🚀

React HackerNews ✅

Timings

Description Time Difference
Cold 4.42s -23.00ms
Cached 403.00ms -12.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/PermalinkedComment.3145598b.js 3.94kb +0.00b 372.00ms -23.00ms 🚀
dist/UserProfile.b37bbaff.js 1.38kb +0.00b 372.00ms -23.00ms 🚀
dist/NotFound.c08212ea.js 265.00b +0.00b 372.00ms -23.00ms 🚀
dist/logo.8dd07848.png 244.00b +0.00b 297.00ms -22.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/logo.8dd07848.png 244.00b +0.00b 335.00ms +30.00ms ⚠️

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 40.45s -826.00ms
Cached 2.39s +79.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/card.3521c96b.js 140.18kb +0.00b 11.98s +2.57s ⚠️
dist/esm.ce3e12df.js 59.72kb +0.00b 11.98s +2.57s ⚠️
dist/pdfRenderer.6335b9a2.js 12.08kb +0.00b 13.11s +3.71s ⚠️
dist/mobile-upload.e9eb996a.js 7.86kb +0.00b 11.98s +2.57s ⚠️
dist/codeViewerRenderer.7d374cd5.js 2.61kb +0.00b 13.26s +3.86s ⚠️
dist/pl.f089a702.js 2.25kb +0.00b 7.95s +1.22s ⚠️
dist/ja.a9cd0bd6.js 2.09kb +0.00b 7.95s +1.22s ⚠️
dist/pt_BR.1db6fd92.js 2.06kb +0.00b 7.95s +1.23s ⚠️
dist/ko.954590a1.js 1.97kb +0.00b 7.95s +1.22s ⚠️
dist/nb.7f52770f.js 1.96kb +0.00b 7.95s +1.22s ⚠️
dist/nl.fd54481e.js 1.94kb +0.00b 7.95s +1.22s ⚠️
dist/workerHasher.8fdadeba.js 1.56kb +0.00b 11.98s +2.57s ⚠️
dist/pt_PT.16308ef8.js 631.00b +0.00b 7.95s +1.23s ⚠️
dist/simpleHasher.180c1d91.js 585.00b +0.00b 11.98s +2.57s ⚠️
dist/index.html 248.00b +0.00b 13.32s +6.61s ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/component-lazy.aeb22f50.js 59.50kb +0.00b 6.88s +477.00ms ⚠️

Three.js ✅

Timings

Description Time Difference
Cold 3.29s -6.00ms
Cached 358.00ms +8.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/Three.js 572.22kb +0.00b 1.06s -54.00ms 🚀

Click here to view a detailed benchmark overview.

@marcins marcins requested review from devongovett and lettertwo July 31, 2023 23:28
@devongovett
Copy link
Member

This is a cool feature! My only feedback is that the naming of the CLI options is a bit confusing potentially. e.g. does "include" mean "in addition to the default" or "only these ones". Maybe we could make it so --lazy does what it does now, but --lazy "some glob" works like you have implemented here (only these files are lazy). And keep --lazy-exclude also I guess.

@marcins
Copy link
Contributor Author

marcins commented Aug 22, 2023

This is a cool feature! My only feedback is that the naming of the CLI options is a bit confusing potentially. e.g. does "include" mean "in addition to the default" or "only these ones". Maybe we could make it so --lazy does what it does now, but --lazy "some glob" works like you have implemented here (only these files are lazy). And keep --lazy-exclude also I guess.

Thanks for this feedback - I've now had a chance to revise this.

@alshdavid alshdavid self-requested a review August 25, 2023 03:57
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.

4 participants