parcel icon indicating copy to clipboard operation
parcel copied to clipboard

Add side effect detection visitor

Open mattcompiles opened this issue 2 years ago • 3 comments

↪️ Pull Request

This is a first pass on a side effect detecting visitor that can upgrade assets in an unknown side effect state to sideEffects: false in some cases.

Running it on a very large code base, it detected 33% of assets as side effect free. I haven't found any false positives but feedback and more test coverage should give more confidence.

Major changes in this PR

  • Added a new side effects visitor to the core JS transformer
    • Only runs in scope hoisting mode (due to being reliant on import local names)
    • Only runs when the Asset is in an unknown side effects state
  • Added a new property (hasResolvedSideEffects) to Assets to be able to distinguish between sideEffects: true and sideEffects: undefined
    • The new property allows for backwards compatibility
  • Updates node-resolver-core to only return side effects in known situations, otherwise it will be undefined.
    • Previously we were defaulting in both the resolver and on the core Asset

💻 Examples

🚨 Test instructions

✔️ PR Todo

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

mattcompiles avatar Aug 18 '23 06:08 mattcompiles

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.51s +8.00ms
Cached 273.00ms -24.00ms 🚀

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 243.00ms +13.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 251.00ms +15.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 251.00ms +14.00ms ⚠️
dist/legacy/index.b8ae99ba.css 94.00b +0.00b 252.00ms +15.00ms ⚠️
dist/modern/index.31cedca9.css 94.00b +0.00b 252.00ms +15.00ms ⚠️

Cached Bundles

No bundle changes detected.

React HackerNews ✅

Timings

Description Time Difference
Cold 4.11s -67.00ms
Cached 376.00ms -2.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/PermalinkedComment.3145598b.js 3.94kb +0.00b 420.00ms +56.00ms ⚠️
dist/UserProfile.b37bbaff.js 1.38kb +0.00b 419.00ms +55.00ms ⚠️
dist/NotFound.c08212ea.js 265.00b +0.00b 420.00ms +56.00ms ⚠️
dist/logo.8dd07848.png 244.00b +0.00b 326.00ms +24.00ms ⚠️
AtlasKit Editor ✅

Timings

Description Time Difference
Cold 37.87s -1.15s
Cached 2.18s -0.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/media-viewer.bc1a2415.js 537.32kb +0.00b 11.81s +2.59s ⚠️
dist/ConfigPanelFieldsLoader.8648eeee.js 306.81kb +0.00b 8.31s -911.00ms 🚀
dist/card.3521c96b.js 140.18kb +0.00b 8.31s -910.00ms 🚀
dist/mobile-upload.0917d4f0.js 66.50kb +0.00b 5.31s -285.00ms 🚀
dist/ElementBrowser.c496dd44.js 62.20kb +0.00b 8.31s -910.00ms 🚀
dist/archive.fe044de4.js 60.16kb +0.00b 11.81s +2.59s ⚠️
dist/esm.ce3e12df.js 59.72kb +0.00b 8.32s -900.00ms 🚀
dist/component-lazy.aeb22f50.js 59.50kb +0.00b 6.24s +337.00ms ⚠️
dist/component.508d12ab.js 57.88kb +0.00b 5.30s -291.00ms 🚀
dist/DatePicker.f4cb448f.js 47.85kb +0.00b 6.25s -360.00ms 🚀
dist/Modal.f90b31a7.js 28.20kb +0.00b 5.31s -270.00ms 🚀
dist/DatePicker.412226fe.js 25.02kb +0.00b 6.25s -360.00ms 🚀
dist/component.d038388b.js 18.68kb +0.00b 5.30s -275.00ms 🚀
dist/js.9cb9c5be.js 17.21kb +0.00b 5.30s -275.00ms 🚀
dist/ConfigPanelFieldsLoader.8efb299e.js 15.82kb +0.00b 8.31s -911.00ms 🚀
dist/ui.8e1e1200.js 14.49kb +0.00b 8.31s -910.00ms 🚀
dist/ConfigPanelFieldsLoader.f78f3b60.js 13.65kb +0.00b 8.31s -910.00ms 🚀
dist/pdfRenderer.6335b9a2.js 12.08kb +0.00b 11.56s +2.35s ⚠️
dist/mobile-upload.86840439.js 7.86kb +0.00b 5.30s -272.00ms 🚀
dist/mobile-upload.c687ddb2.js 7.86kb +0.00b 8.31s -911.00ms 🚀
dist/mobile-upload.e9eb996a.js 7.86kb +0.00b 8.32s -902.00ms 🚀
dist/Modal.efe95f7f.js 3.87kb +0.00b 5.30s -275.00ms 🚀
dist/component.342752e9.js 3.22kb +0.00b 5.30s -275.00ms 🚀
dist/media-viewer-analytics-error-boundary.54c54975.js 3.19kb +0.00b 11.81s +2.59s ⚠️
dist/png-chunks-extract.01ed8f60.js 3.06kb +0.00b 5.30s -272.00ms 🚀
dist/ru.aaea8ba6.js 2.81kb +0.00b 8.31s +1.70s ⚠️
dist/uk.5d2e97bd.js 2.76kb +0.00b 8.31s -912.00ms 🚀
dist/codeViewerRenderer.7d374cd5.js 2.61kb +0.00b 11.79s +2.57s ⚠️
dist/th.df60823c.js 2.60kb +0.00b 8.31s -912.00ms 🚀
dist/ResourcedEmojiComponent.184d62aa.js 2.47kb +0.00b 6.25s -359.00ms 🚀
dist/pl.f089a702.js 2.25kb +0.00b 6.25s -360.00ms 🚀
dist/cs.c0d356c1.js 2.23kb +0.00b 6.25s -360.00ms 🚀
dist/de.1a167b65.js 2.17kb +0.00b 6.25s -360.00ms 🚀
dist/fr.6cc5b166.js 2.12kb +0.00b 6.25s -359.00ms 🚀
dist/es.38a88442.js 2.12kb +0.00b 6.25s -360.00ms 🚀
dist/hu.026ff8dd.js 2.10kb +0.00b 6.25s -359.00ms 🚀
dist/fi.84541eb7.js 2.09kb +0.00b 6.25s -359.00ms 🚀
dist/ja.a9cd0bd6.js 2.09kb +0.00b 6.25s -360.00ms 🚀
dist/vi.3e6d5bcb.js 2.09kb +0.00b 8.31s -911.00ms 🚀
dist/pt_BR.1db6fd92.js 2.06kb +0.00b 7.65s +1.04s ⚠️
dist/tr.4de346b9.js 2.03kb +0.00b 8.31s -912.00ms 🚀
dist/ko.954590a1.js 1.97kb +0.00b 6.25s -360.00ms 🚀
dist/sv.b893ead3.js 1.97kb +0.00b 8.31s -911.00ms 🚀
dist/it.5c7edaaf.js 1.97kb +0.00b 6.25s -360.00ms 🚀
dist/nb.7f52770f.js 1.96kb +0.00b 6.25s -360.00ms 🚀
dist/date.6db71354.js 1.94kb +0.00b 5.58s -315.00ms 🚀
dist/da.23f674ea.js 1.94kb +0.00b 6.25s -360.00ms 🚀
dist/nl.fd54481e.js 1.94kb +0.00b 6.25s -360.00ms 🚀
dist/images.21df3a8f.js 1.90kb +0.00b 5.58s -315.00ms 🚀
dist/zh_TW.3d130b76.js 1.85kb +0.00b 8.31s -911.00ms 🚀
dist/zh.fb21f066.js 1.83kb +0.00b 8.31s -911.00ms 🚀
dist/feedback.647089cf.js 1.76kb +0.00b 6.25s -361.00ms 🚀
dist/status.be4e3842.js 1.67kb +0.00b 5.58s -315.00ms 🚀
dist/code.64a301f3.js 1.56kb +0.00b 5.58s -315.00ms 🚀
dist/workerHasher.e01f8bcf.js 1.56kb +0.00b 5.30s -272.00ms 🚀
dist/workerHasher.322762e4.js 1.56kb +0.00b 8.31s -910.00ms 🚀
dist/workerHasher.8fdadeba.js 1.56kb +0.00b 8.32s -903.00ms 🚀
dist/list-number.e454dc8e.js 1.47kb +0.00b 5.58s -316.00ms 🚀
dist/heading6.eae34279.js 1.36kb +0.00b 6.25s -361.00ms 🚀
dist/16.a4c7368c.js 1.35kb +0.00b 5.31s -272.00ms 🚀
dist/heading3.82217cc7.js 1.35kb +0.00b 5.58s -316.00ms 🚀
dist/16.347f2ad3.js 1.28kb +0.00b 5.30s -275.00ms 🚀
dist/link.ef87b7d4.js 1.28kb +0.00b 5.58s -316.00ms 🚀
dist/emoji.f9caa19f.js 1.25kb +0.00b 5.58s -315.00ms 🚀
dist/heading5.20183aa6.js 1.23kb +0.00b 6.25s -360.00ms 🚀
dist/expand.e7437f2e.js 1.18kb +0.00b 6.25s -359.00ms 🚀
dist/heading2.a43a84af.js 1.17kb +0.00b 5.58s -316.00ms 🚀
dist/heading4.bc1ea347.js 1.12kb +0.00b 5.58s -315.00ms 🚀
dist/mention.12d040af.js 1.08kb +0.00b 5.58s -315.00ms 🚀
dist/layout.467fc22b.js 1.04kb +0.00b 5.58s -316.00ms 🚀
dist/divider.875eeb9b.js 1.04kb +0.00b 5.58s -315.00ms 🚀
dist/action.4747cf93.js 1.02kb +0.00b 5.58s -315.00ms 🚀
dist/heading1.a2a2d506.js 1.01kb +0.00b 5.58s -316.00ms 🚀
dist/16.1d939d76.js 1.00kb +0.00b 5.30s -276.00ms 🚀
dist/list.a024d070.js 1007.00b +0.00b 5.58s -316.00ms 🚀
dist/quote.790784b0.js 1007.00b +0.00b 5.58s -315.00ms 🚀
dist/decision.f09ec841.js 988.00b +0.00b 5.58s -315.00ms 🚀
dist/16.92be0d97.js 976.00b +0.00b 5.30s -275.00ms 🚀
dist/16.5befcdea.js 976.00b +0.00b 5.30s -275.00ms 🚀
dist/panel-warning.b246d7d3.js 964.00b +0.00b 5.58s -315.00ms 🚀
dist/16.ef2df2b6.js 956.00b +0.00b 5.30s -275.00ms 🚀
dist/16.792a9556.js 951.00b +0.00b 5.58s -316.00ms 🚀
dist/table.348d2eb0.js 942.00b +0.00b 5.58s -316.00ms 🚀
dist/16.01cdc55d.js 916.00b +0.00b 5.31s -285.00ms 🚀
dist/panel.bc621c8f.js 883.00b +0.00b 5.58s -315.00ms 🚀
dist/panel-error.9152f129.js 860.00b +0.00b 5.58s -315.00ms 🚀
dist/16.8fc349c9.js 858.00b +0.00b 5.31s -271.00ms 🚀
dist/16.c5423dbe.js 830.00b +0.00b 5.31s -285.00ms 🚀
dist/16.1be49b4b.js 823.00b +0.00b 5.30s -276.00ms 🚀
dist/16.eb6f51c1.js 817.00b +0.00b 5.58s -316.00ms 🚀
dist/panel-success.25629fed.js 801.00b +0.00b 5.58s -315.00ms 🚀
dist/panel-note.f23dd251.js 791.00b +0.00b 5.58s -315.00ms 🚀
dist/16.924dd2e3.js 778.00b +0.00b 5.30s -275.00ms 🚀
dist/16.01e372d3.js 772.00b +0.00b 5.30s -275.00ms 🚀
dist/16.0cd21a5f.js 772.00b +0.00b 5.30s -275.00ms 🚀
dist/16.a0490963.js 771.00b +0.00b 5.30s -277.00ms 🚀
dist/16.a5f45cdb.js 770.00b +0.00b 5.30s -275.00ms 🚀
dist/16.bc1a05f3.js 769.00b +0.00b 5.30s -276.00ms 🚀
dist/16.d9cd1f88.js 742.00b +0.00b 5.58s -315.00ms 🚀
dist/16.e3d16653.js 721.00b +0.00b 5.30s -276.00ms 🚀
dist/16.592b5fd3.js 693.00b +0.00b 5.31s -285.00ms 🚀
dist/sk.1a0c584e.js 652.00b +0.00b 8.31s -910.00ms 🚀
dist/pt_PT.16308ef8.js 631.00b +0.00b 7.65s +1.03s ⚠️
dist/et.3d28125f.js 629.00b +0.00b 6.25s -360.00ms 🚀
dist/simpleHasher.329400f6.js 585.00b +0.00b 5.30s -272.00ms 🚀
dist/simpleHasher.0488d56a.js 585.00b +0.00b 8.31s -911.00ms 🚀
dist/simpleHasher.180c1d91.js 585.00b +0.00b 8.32s -903.00ms 🚀
dist/is.b5f0121f.js 491.00b +0.00b 6.25s -360.00ms 🚀
dist/ro.ee42c980.js 478.00b +0.00b 8.23s +1.61s ⚠️
dist/en_GB.f6c48dd5.js 468.00b +0.00b 6.25s -360.00ms 🚀
dist/en.b8f14ffb.js 465.00b +0.00b 6.25s -360.00ms 🚀
dist/index.html 248.00b +0.00b 11.96s +5.41s ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/media-viewer.bc1a2415.js 537.32kb +0.00b 9.16s -3.28s 🚀
dist/archive.fe044de4.js 60.16kb +0.00b 9.16s -3.28s 🚀
dist/index.html 248.00b +0.00b 6.47s -5.99s 🚀
Three.js ✅

Timings

Description Time Difference
Cold 3.15s +16.00ms
Cached 313.00ms -5.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

parcel-benchmark avatar Aug 18 '23 07:08 parcel-benchmark

@marcins On the webpack tests, I actually couldn't find any tests for their logic. 🤷

mattcompiles avatar Aug 22 '23 22:08 mattcompiles

@mattcompiles We discussed with @mischnic in the meeting today. Sounds like we will need a new value/mode for asset.sideEffects that indicates that the side effects only apply to that asset and not to any dependencies. The packager already has the ability to skip an asset while preserving the asset's dependencies (shouldSkipAsset), and also a mode that skips entire subgraphs (isDependencySkipped). I think we just need something similar in symbol propagation.

devongovett avatar Sep 12 '23 17:09 devongovett