Improve performance of incremental bundling #8583
Conversation
| return graph; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
These cases were never hit at all in the tests, and should be very rare. I think removing the traversal to compute the hashes in the common case outweighs the potential benefits for zero change builds.
Benchmark ResultsKitchen Sink ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. React HackerNews 🚨
Timings
Cold BundlesNo bundles found, this is probably a failed build... Cached BundlesNo bundles found, this is probably a failed build... AtlasKit Editor 🚨
Timings
Cold BundlesNo bundles found, this is probably a failed build... Cached BundlesNo bundles found, this is probably a failed build... Three.js ✅
Timings
Cold BundlesNo bundle changes detected. Cached Bundles
|
Did you commit that? I don't see where that's done |
|
Yeah, the getCacheKey function was deleted and replaced with this.cacheKey |
|
But neither RequestGraph nor RequestTracker contain changes? Did you mean
|
|
Oh, oops, yes that is exactly what I meant. 🤦 |
| // If any subrequests are invalid (e.g. dev dep requests or config requests), | ||
| // bail on incremental bundling. We also need to invalidate for option changes, | ||
| // which are hoisted to direct invalidations on the bundle graph request. | ||
| let subRequestsInvalid = |
There was a problem hiding this comment.
Does this mean that changes like adding a runtime would stop incremental bundling from happening? Thats the only issue I encountered when trying to not apply runtimes for incremental.
There was a problem hiding this comment.
Yeah, if any config changes or plugin changes occurred, then bundling will not be incremental. I think this should be pretty rare so probably acceptable?
This is a refactor of #6514, which improves performance even more.