Skip to content

Commit

Permalink
Dynamically determine BundleGroups with references from the BundleGra…
Browse files Browse the repository at this point in the history 
…ph (#5200)

* Add test

* Add another case

* Use set instead of array

* ¯\_(ツ)_/¯

* Fix sibling bundle worker test case

* Add test case for shared sibling bundles with a common dependency

* Initial commit of asset siblings in graph

* Use reference edge for asset bundle references and replace bundle reference edges

* Use sibling edges instead of bundle edges for siblings

* Replace getReferencedBundles with recursive getSiblingBundles

* Use reference edges instead of sibling edges

* Use bundle to bundle reference edges for shared bundles

* Default to non-recursive getReferencedBundles

* Don't consider siblings if the dependency points to a bundle group

* Remove bundleGraph.isAssetReferenced

* Create asset references with associated destination bundles

* getSiblingBundles => getReferencedBundles

* Remove getBundleWithAssetAsEntry

* Default getReferencedBundles to recursive

* getBundleGroupsContainingBundle: use function stack instead of array stack

* Define and use getReferencingBundles in isAssetReferencedByDependant

* Retrieve referenced bundles in the opposite order they were added

* Remove bundleGroup.bundleIds list

* Handle unstable sort in node 10

* Use graph.dfs() for getReferencedBundles

* Use graph.traverseAncestors() for getReferencingBundles

* Implement bundleGraph.getParentBundles() in terms of getBundleGroupsContainingBundle() and getParentBundlesOfBundleGroup()

* Add multiple edge types to traverseAncestors

* findReachableBundleWithAsset: traverse along reference edges as well

* Traverse reference edges as well in isAssetReachableFromBundle

* Don't recursively check reachability when removing asset graphs

* Simplify bundleGraph.isAssetReferencedByDependant()

Co-authored-by: Niklas Mischkulnig <mischnic@users.noreply.github.com>
  • Loading branch information
@mischnic
Will Binns-Smith and mischnic committed Dec 18, 2020
1 parent d77a541 commit 3f1935a
Show file tree
Hide file tree
Showing 33 changed files with 415 additions and 275 deletions.
71 changes: 7 additions & 64 deletions packages/bundlers/default/src/DefaultBundler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export default (new Bundler({
bundle({bundleGraph}) {
let bundleRoots: Map<Bundle, Array<Asset>> = new Map();
let bundlesByEntryAsset: Map<Asset, Bundle> = new Map();
let siblingBundlesByAsset: Map<string, Array<Bundle>> = new Map();

// Step 1: create bundles for each of the explicit code split points.
bundleGraph.traverse({
Expand Down Expand Up @@ -110,7 +109,6 @@ export default (new Bundler({
});
bundleByType.set(bundle.type, bundle);
bundlesByEntryAsset.set(asset, bundle);
siblingBundlesByAsset.set(asset.id, []);
bundleGraph.addBundleToBundleGroup(bundle, bundleGroup);

// The bundle may have already been created, and the graph gave us back the original one...
Expand All @@ -119,14 +117,9 @@ export default (new Bundler({
}

// If the bundle is in the same bundle group as the parent, create an asset reference
// between the dependency and the asset, and a bundle reference between the parent bundle
// and the child bundle.
// between the dependency, the asset, and the target bundle.
if (bundleGroup === context?.bundleGroup) {
bundleGraph.createAssetReference(dependency, asset);
bundleGraph.createBundleReference(
nullthrows(context?.parentBundle),
bundle,
);
bundleGraph.createAssetReference(dependency, asset, bundle);
}
}

Expand All @@ -145,35 +138,9 @@ export default (new Bundler({
let parentBundle = context.parentBundle;
let bundleGroup = nullthrows(context.bundleGroup);
let bundleByType = nullthrows(context.bundleByType);
let siblingBundles = nullthrows(
siblingBundlesByAsset.get(parentAsset.id),
);
let allSameType = assets.every(a => a.type === parentAsset.type);

for (let asset of assets) {
let siblings = siblingBundlesByAsset.get(asset.id);

if (parentAsset.type === asset.type) {
if (allSameType && siblings) {
// If any sibling bundles were created for this asset or its subtree previously,
// add them all to the current bundle group as well. This fixes cases where two entries
// depend on a shared asset which has siblings. Due to DFS, the subtree of the shared
// asset is only processed once, meaning any sibling bundles created due to type changes
// would only be connected to the first bundle group. To work around this, we store a list
// of sibling bundles for each asset in the graph, and when we re-visit a shared asset, we
// connect them all to the current bundle group as well.
for (let bundle of siblings) {
bundleGraph.addBundleToBundleGroup(bundle, bundleGroup);
}
} else if (!siblings) {
// Propagate the same siblings further if there are no bundles being created in this
// asset group, otherwise start a new set of siblings.
siblingBundlesByAsset.set(
asset.id,
allSameType ? siblingBundles : [],
);
}

continue;
}

Expand All @@ -183,7 +150,7 @@ export default (new Bundler({
// merge this subgraph into it.
nullthrows(bundleRoots.get(existingBundle)).push(asset);
bundlesByEntryAsset.set(asset, existingBundle);
bundleGraph.createAssetReference(dependency, asset);
bundleGraph.createAssetReference(dependency, asset, existingBundle);
} else {
let bundle = bundleGraph.createBundle({
uniqueKey: asset.id,
Expand All @@ -199,21 +166,14 @@ export default (new Bundler({
pipeline: asset.pipeline,
});
bundleByType.set(bundle.type, bundle);
siblingBundles.push(bundle);
bundlesByEntryAsset.set(asset, bundle);
bundleGraph.createAssetReference(dependency, asset);
bundleGraph.createBundleReference(parentBundle, bundle);
bundleGraph.addBundleToBundleGroup(bundle, bundleGroup);
bundleGraph.createAssetReference(dependency, asset, bundle);

// The bundle may have already been created, and the graph gave us back the original one...
if (!bundleRoots.has(bundle)) {
bundleRoots.set(bundle, [asset]);
}
}

if (!siblings) {
siblingBundlesByAsset.set(asset.id, []);
}
}

return {
Expand Down Expand Up @@ -251,9 +211,6 @@ export default (new Bundler({
return;
}

let siblings = bundleGraph
.getReferencedBundles(bundle)
.filter(sibling => !sibling.isInline);
let candidates = bundleGraph.findBundlesWithAsset(mainEntry).filter(
containingBundle =>
containingBundle.id !== bundle.id &&
Expand All @@ -277,12 +234,8 @@ export default (new Bundler({
.filter(b => !b.isInline).length < config.maxParallelRequests,
)
) {
bundleGraph.createBundleReference(candidate, bundle);
bundleGraph.removeAssetGraphFromBundle(mainEntry, candidate);
for (let bundleGroup of bundleGroups) {
for (let bundleToAdd of [bundle, ...siblings]) {
bundleGraph.addBundleToBundleGroup(bundleToAdd, bundleGroup);
}
}
}
}
});
Expand Down Expand Up @@ -391,19 +344,8 @@ export default (new Bundler({
type: firstBundle.type,
});

// Create new bundle node and connect it to all of the original bundle groups
for (let bundleGroup of bundleGroups) {
// If the bundle group is within the parallel request limit, then add the shared bundle.
if (
bundleGraph
.getBundlesInBundleGroup(bundleGroup)
.filter(b => !b.isInline).length < config.maxParallelRequests
) {
bundleGraph.addBundleToBundleGroup(sharedBundle, bundleGroup);
}
}

// Remove all of the root assets from each of the original bundles
// and reference the new shared bundle.
for (let asset of assets) {
bundleGraph.addAssetGraphToBundle(asset, sharedBundle);

Expand All @@ -421,6 +363,7 @@ export default (new Bundler({
.filter(b => !b.isInline).length < config.maxParallelRequests,
)
) {
bundleGraph.createBundleReference(bundle, sharedBundle);
bundleGraph.removeAssetGraphFromBundle(asset, bundle);
}
}
Expand Down
Loading

0 comments on commit 3f1935a

Please sign in to comment.