From ee19e03d7bf8f880c88962d106ba2e954c8312b4 Mon Sep 17 00:00:00 2001 From: Agnieszka Gawrys Date: Wed, 11 Oct 2023 10:43:50 -0700 Subject: [PATCH 1/2] clean up msb code for code split bundle creation --- .../bundlers/default/src/DefaultBundler.js | 146 ++++++------------ 1 file changed, 47 insertions(+), 99 deletions(-) diff --git a/packages/bundlers/default/src/DefaultBundler.js b/packages/bundlers/default/src/DefaultBundler.js index 82dccc5f75c..6f7ef64db2e 100644 --- a/packages/bundlers/default/src/DefaultBundler.js +++ b/packages/bundlers/default/src/DefaultBundler.js @@ -561,28 +561,29 @@ function createIdealGraph( // MSB Step 1: Match glob on filepath and type for any asset let manualSharedBundleKey; let manualSharedObject = manualAssetToConfig.get(childAsset); - + let bundleId; + let bundle; if (manualSharedObject) { // MSB Step 2: Generate a key for which to look up this manual bundle with manualSharedBundleKey = manualSharedObject.name + ',' + childAsset.type; } + + if ( + // MSB Step 3: If a bundle for these globs already exsits, use it + manualSharedBundleKey != null && + manualSharedMap.has(manualSharedBundleKey) + ) { + bundleId = nullthrows(manualSharedMap.get(manualSharedBundleKey)); + } if ( dependency.priority === 'lazy' || childAsset.bundleBehavior === 'isolated' // An isolated Dependency, or Bundle must contain all assets it needs to load. ) { - let bundleId = bundles.get(childAsset.id); - let bundle; - - if ( - // MSB Step 3: If a bundle for these globs already exsits, use it - manualSharedBundleKey != null && - manualSharedMap.has(manualSharedBundleKey) - ) { - bundleId = nullthrows( - manualSharedMap.get(manualSharedBundleKey), - ); + if (bundleId == null) { + bundleId = bundles.get(childAsset.id); } + if (bundleId == null) { let firstBundleGroup = nullthrows( bundleGraph.getNode(stack[0][1]), @@ -605,20 +606,11 @@ function createIdealGraph( bundleGroupBundleIds.add(bundleId); bundleGraph.addEdge(bundleGraphRootNodeId, bundleId); if (manualSharedObject) { - // MSB Step 4: If this is the first instance of a match, set it appropriately - //If this was an existing glob we must add the asset - manualAssetToBundle.set(childAsset, bundleId); // Add asset to bundle - - invariant(bundle !== 'root' && bundle !== null); - bundle.size = childAsset.stats.size; - invariant(manualSharedBundleKey != null); - if (!manualSharedMap.has(manualSharedBundleKey)) { - manualSharedMap.set(manualSharedBundleKey, bundleId); - } + // MSB Step 4: If this was the first instance of a match, mark mainAsset for internalization + // since MSBs should not have main entry assets nullthrows( manualBundleToInternalizedAsset.get(bundleId), ).push(childAsset); - bundle.manualSharedBundle = manualSharedObject.name; } } else { bundle = nullthrows(bundleGraph.getNode(bundleId)); @@ -632,32 +624,6 @@ function createIdealGraph( ) { bundle.bundleBehavior = dependency.bundleBehavior; } - if (manualSharedObject) { - // MSB Step 5: If a bundle for this asset already exists and we have a glob match - // simply add the asset if it doesn't already have it - // If this was an existing glob we must add the asset - manualAssetToBundle.set(childAsset, bundleId); // Add asset to bundle - - invariant(bundle !== 'root' && bundle !== null); - - if (!bundle.assets.has(childAsset)) { - bundle.assets.add(childAsset); - bundle.size += childAsset.stats.size; - } - - bundles.set(childAsset.id, bundleId); - bundleRoots.set(childAsset, [bundleId, bundleId]); - - nullthrows( - manualBundleToInternalizedAsset.get(bundleId), - ).push(childAsset); - - invariant(manualSharedBundleKey != null); - if (!manualSharedMap.has(manualSharedBundleKey)) { - manualSharedMap.set(manualSharedBundleKey, bundleId); - } - bundle.manualSharedBundle = manualSharedObject.name; - } } dependencyBundleGraph.addEdge( @@ -677,9 +643,7 @@ function createIdealGraph( ), dependencyPriorityEdges[dependency.priority], ); - continue; - } - if ( + } else if ( parentAsset.type !== childAsset.type || dependency.priority === 'parallel' || childAsset.bundleBehavior === 'inline' @@ -693,7 +657,6 @@ function createIdealGraph( ); invariant(bundleGroup !== 'root'); - let bundleId; let referencingBundleId = nullthrows( bundleRoots.get(referencingBundleRoot), )[0]; @@ -701,9 +664,9 @@ function createIdealGraph( bundleGraph.getNode(referencingBundleId), ); invariant(referencingBundle !== 'root'); - let bundle; - bundleId = bundles.get(childAsset.id); - + if (bundleId == null) { + bundleId = bundles.get(childAsset.id); + } /** * If this is an entry bundlegroup, we only allow one bundle per type in those groups * So attempt to add the asset to the entry bundle if it's of the same type. @@ -715,21 +678,11 @@ function createIdealGraph( parentAsset.type !== childAsset.type && entries.has(bundleGroupRootAsset) && canMerge(bundleGroupRootAsset, childAsset) && - dependency.bundleBehavior == null + dependency.bundleBehavior == null && + manualSharedBundleKey == null //exclude MSBs for merging ) { bundleId = bundleGroupNodeId; } - if ( - // MSB Step 3 alt: If a bundle for these globs already exsits, use it - manualSharedBundleKey != null && - manualSharedMap.has(manualSharedBundleKey) - ) { - // If theres an existing bundle for this glob, add the asset - bundleId = nullthrows( - manualSharedMap.get(manualSharedBundleKey), - ); - bundle = nullthrows(bundleGraph.getNode(bundleId)); - } if (bundleId == null) { bundle = createBundle({ // Bundles created from type changes shouldn't have an entry asset. @@ -753,20 +706,6 @@ function createIdealGraph( if (parentAsset.type !== childAsset.type) { typeChangeIds.add(bundleId); } - if (manualSharedObject) { - //If this was an existing glob we must add the asset - manualAssetToBundle.set(childAsset, bundleId); // Add asset to bundle - - invariant(bundle !== 'root' && bundle !== null); - bundle.assets.add(childAsset); - bundle.size += childAsset.stats.size; - bundles.set(childAsset.id, bundleId); // bundleRoots.set(childAsset, [bundleId, bundleId]); - invariant(manualSharedBundleKey != null); - if (!manualSharedMap.has(manualSharedBundleKey)) { - manualSharedMap.set(manualSharedBundleKey, bundleId); - } - bundle.manualSharedBundle = manualSharedObject.name; - } } else { bundle = bundleGraph.getNode(bundleId); invariant(bundle != null && bundle !== 'root'); @@ -780,22 +719,6 @@ function createIdealGraph( bundle.bundleBehavior = dependency.bundleBehavior; } } - // GLOB MATCHING - If the bundle did exist, added the new asset, if the bundle was jsut created, add the key to map - if (manualSharedObject) { - // If this was an existing glob we must add the asset - manualAssetToBundle.set(childAsset, bundleId); // Add asset to bundle - - invariant(bundle !== 'root' && bundle !== null); - bundle.assets.add(childAsset); - bundle.size += childAsset.stats.size; - bundles.set(childAsset.id, bundleId); - - invariant(manualSharedBundleKey != null); - if (!manualSharedMap.has(manualSharedBundleKey)) { - manualSharedMap.set(manualSharedBundleKey, bundleId); - } - bundle.manualSharedBundle = manualSharedObject.name; - } bundles.set(childAsset.id, bundleId); @@ -826,7 +749,32 @@ function createIdealGraph( } assetReference.get(childAsset).push([dependency, bundle]); - continue; + } + if (manualSharedObject && bundleId != null) { + // MSB Step 5: At this point we've either created or found an existing MSB bundle + // add the asset if it doesn't already have it and set key + + invariant( + bundle !== 'root' && bundle != null && bundleId != null, + ); + + manualAssetToBundle.set(childAsset, bundleId); + + if (!bundle.assets.has(childAsset)) { + // Add asset to bundle + bundle.assets.add(childAsset); + bundle.size += childAsset.stats.size; + } + + bundles.set(childAsset.id, bundleId); + bundleRoots.set(childAsset, [bundleId, bundleId]); + + invariant(manualSharedBundleKey != null); + // Ensure we set key to BundleId so the next glob match uses the appropriate bundle + if (!manualSharedMap.has(manualSharedBundleKey)) { + manualSharedMap.set(manualSharedBundleKey, bundleId); + } + bundle.manualSharedBundle = manualSharedObject.name; } } } From 9eedd55f47601751464ba2ff2065e6bab73b1edf Mon Sep 17 00:00:00 2001 From: Agnieszka Gawrys Date: Tue, 17 Oct 2023 13:43:25 -0700 Subject: [PATCH 2/2] fix case where dep is not special but bundle existed from another source --- .../bundlers/default/src/DefaultBundler.js | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/bundlers/default/src/DefaultBundler.js b/packages/bundlers/default/src/DefaultBundler.js index 6f7ef64db2e..ccf86addb70 100644 --- a/packages/bundlers/default/src/DefaultBundler.js +++ b/packages/bundlers/default/src/DefaultBundler.js @@ -509,7 +509,7 @@ function createIdealGraph( let manualAssetToBundle: Map = new Map(); let {manualAssetToConfig, constantModuleToMSB} = makeManualAssetToConfigLookup(); - let manualBundleToInternalizedAsset: Map< + let manualBundleToInternalizedAsset: DefaultMap< NodeId, Array, > = new DefaultMap(() => []); @@ -558,11 +558,13 @@ function createIdealGraph( } for (let childAsset of assets) { + let bundleId = bundles.get(childAsset.id); + let bundle; + // MSB Step 1: Match glob on filepath and type for any asset let manualSharedBundleKey; let manualSharedObject = manualAssetToConfig.get(childAsset); - let bundleId; - let bundle; + if (manualSharedObject) { // MSB Step 2: Generate a key for which to look up this manual bundle with manualSharedBundleKey = @@ -580,10 +582,6 @@ function createIdealGraph( dependency.priority === 'lazy' || childAsset.bundleBehavior === 'isolated' // An isolated Dependency, or Bundle must contain all assets it needs to load. ) { - if (bundleId == null) { - bundleId = bundles.get(childAsset.id); - } - if (bundleId == null) { let firstBundleGroup = nullthrows( bundleGraph.getNode(stack[0][1]), @@ -608,9 +606,9 @@ function createIdealGraph( if (manualSharedObject) { // MSB Step 4: If this was the first instance of a match, mark mainAsset for internalization // since MSBs should not have main entry assets - nullthrows( - manualBundleToInternalizedAsset.get(bundleId), - ).push(childAsset); + manualBundleToInternalizedAsset + .get(bundleId) + .push(childAsset); } } else { bundle = nullthrows(bundleGraph.getNode(bundleId)); @@ -664,9 +662,7 @@ function createIdealGraph( bundleGraph.getNode(referencingBundleId), ); invariant(referencingBundle !== 'root'); - if (bundleId == null) { - bundleId = bundles.get(childAsset.id); - } + /** * If this is an entry bundlegroup, we only allow one bundle per type in those groups * So attempt to add the asset to the entry bundle if it's of the same type. @@ -749,6 +745,8 @@ function createIdealGraph( } assetReference.get(childAsset).push([dependency, bundle]); + } else { + bundleId = null; } if (manualSharedObject && bundleId != null) { // MSB Step 5: At this point we've either created or found an existing MSB bundle