[Experimental Bundler]: Implement parallel request limits #7863
Conversation
gorakong
changed the title
Benchmark ResultsKitchen Sink ✅
Timings
Cold Bundles
Cached Bundles
React HackerNews ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. AtlasKit Editor ✅
Timings
Cold BundlesNo bundle changes detected. Cached Bundles
Three.js ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. |
gorakong
force-pushed
the
parallel-request-limit
branch
from
575fefd
to
892edd3
Compare
|
confirmed that these changes don't introduce any test regressions 🙂 |
gorakong
force-pushed
the
parallel-request-limit
branch
from
02afc8d
to
4e04010
Compare
|
what's the state of this PR? |
|
@devongovett @wbinnssmith PR has been updated with a Test Plan 🙂 |
|
Would attempting to use default config now allow for the number of bundles to match for the config test? We should get those tests to pass or alter them if we need to dependent on the bundler |
There was a problem hiding this comment.
I think we need a new test for this since I tested with & without the changes on experimental bundler branch and it doesn't seem like theres any difference
- Add a new test testing that changing the parallel request limit changes the bundles produced (maybe this should be in
cache.jssince theres a test already doing something similar for the bundle config, see experimental-bundler-integration branch for updated version of the config tests)
|
@AGawrys Changes have also been included in the |
gorakong
force-pushed
the
parallel-request-limit
branch
from
dcabae4
to
a2e15fc
Compare
gorakong
dismissed
AGawrys’s stale review
Has been resolved -- test has been added.
| bundleIds.add(nodeId); | ||
| }, bundleGroupId); | ||
| bundleGroupToBundles.set(bundleGroupId, bundleIds); | ||
| } |
There was a problem hiding this comment.
Shouldn't all bundles in a bundle group be directly attached to the entry bundle node? When is that not the case?
There was a problem hiding this comment.
It doesn't look like they're connected at this stage yet -- the edges are created in this step of decorateLegacyGraph:
https://github.com/parcel-bundler/parcel/blob/v2/packages/bundlers/experimental/src/ExperimentalBundler.js#L253-L276
|
Closing since superseded by #8303 🙂 |
↪️ Pull Request
Implements parallel request limits in the Experimental Bundler. When creating a shared bundle, only consider source bundles where the bundleGroups they belong to are all within the parallel request limit.
Test Plan
lodashand anotherreact) are removed/merged. (test has already been merged)