Skip to content

Commit 8eb84ee

Browse files
authored
Optimise isAssetReferenced in packager (#858)
1 parent 401e863 commit 8eb84ee

File tree

6 files changed

+240
-3
lines changed

6 files changed

+240
-3
lines changed

.changeset/lovely-pens-love.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@atlaspack/types-internal': minor
3+
'@atlaspack/feature-flags': minor
4+
'@atlaspack/packager-js': minor
5+
'@atlaspack/core': minor
6+
---
7+
8+
Introduce a new `getReferencedAssets(bundle)` method to the BundleGraph to pre-compute referenced assets, this is used by the scope hoisting packager behind a new `precomputeReferencedAssets` feature flag.

packages/core/core/src/BundleGraph.ts

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,158 @@ export default class BundleGraph {
14351435
});
14361436
}
14371437

1438+
// New method: Fast checks only (no caching of results)
1439+
isAssetReferencedFastCheck(bundle: Bundle, asset: Asset): boolean | null {
1440+
// Fast Check #1: If asset is in multiple bundles in same target, it's referenced
1441+
let bundlesWithAsset = this.getBundlesWithAsset(asset).filter(
1442+
(b) =>
1443+
b.target.name === bundle.target.name &&
1444+
b.target.distDir === bundle.target.distDir,
1445+
);
1446+
1447+
if (bundlesWithAsset.length > 1) {
1448+
return true;
1449+
}
1450+
1451+
// Fast Check #2: If asset is referenced by any async/conditional dependency, it's referenced
1452+
let assetNodeId = nullthrows(this._graph.getNodeIdByContentKey(asset.id));
1453+
1454+
if (
1455+
this._graph
1456+
.getNodeIdsConnectedTo(assetNodeId, bundleGraphEdgeTypes.references)
1457+
.map((id) => this._graph.getNode(id))
1458+
.some(
1459+
(node) =>
1460+
node?.type === 'dependency' &&
1461+
(node.value.priority === Priority.lazy ||
1462+
node.value.priority === Priority.conditional) &&
1463+
node.value.specifierType !== SpecifierType.url,
1464+
)
1465+
) {
1466+
return true;
1467+
}
1468+
1469+
// Fast checks failed - return null to indicate expensive computation needed
1470+
return null;
1471+
}
1472+
1473+
getReferencedAssets(bundle: Bundle): Set<Asset> {
1474+
let referencedAssets = new Set<Asset>();
1475+
1476+
// Build a map of all assets in this bundle with their dependencies
1477+
// This allows us to check all assets in a single traversal
1478+
let assetDependenciesMap = new Map<Asset, Array<Dependency>>();
1479+
1480+
this.traverseAssets(bundle, (asset) => {
1481+
// Always do fast checks (no caching)
1482+
let fastCheckResult = this.isAssetReferencedFastCheck(bundle, asset);
1483+
1484+
if (fastCheckResult === true) {
1485+
referencedAssets.add(asset);
1486+
return;
1487+
}
1488+
1489+
// Fast checks failed (fastCheckResult === null), need expensive computation
1490+
// Check if it's actually referenced via traversal
1491+
1492+
// Store dependencies for later batch checking
1493+
let dependencies = this._graph
1494+
.getNodeIdsConnectedTo(
1495+
nullthrows(this._graph.getNodeIdByContentKey(asset.id)),
1496+
)
1497+
.map((id) => nullthrows(this._graph.getNode(id)))
1498+
.filter((node) => node.type === 'dependency')
1499+
.map((node) => {
1500+
invariant(node.type === 'dependency');
1501+
return node.value;
1502+
});
1503+
1504+
if (dependencies.length > 0) {
1505+
assetDependenciesMap.set(asset, dependencies);
1506+
}
1507+
});
1508+
1509+
// If no assets need the expensive check, return early
1510+
if (assetDependenciesMap.size === 0) {
1511+
return referencedAssets;
1512+
}
1513+
1514+
// Get the assets we need to check once
1515+
let assetsToCheck = Array.from(assetDependenciesMap.keys());
1516+
1517+
// Helper function to check if all assets from assetDependenciesMap are in referencedAssets
1518+
const allAssetsReferenced = (): boolean =>
1519+
assetsToCheck.length <= referencedAssets.size &&
1520+
assetsToCheck.every((asset) => referencedAssets.has(asset));
1521+
1522+
// Do ONE traversal to check all remaining assets
1523+
// We can share visitedBundles across all assets because we check every asset
1524+
// against every visited bundle, which matches the original per-asset behavior
1525+
let siblingBundles = new Set(
1526+
this.getBundleGroupsContainingBundle(bundle).flatMap((bundleGroup) =>
1527+
this.getBundlesInBundleGroup(bundleGroup, {includeInline: true}),
1528+
),
1529+
);
1530+
1531+
let visitedBundles: Set<Bundle> = new Set();
1532+
1533+
// Single traversal from all referencers
1534+
for (let referencer of siblingBundles) {
1535+
this.traverseBundles((descendant, _, actions) => {
1536+
if (descendant.id === bundle.id) {
1537+
return;
1538+
}
1539+
1540+
if (visitedBundles.has(descendant)) {
1541+
actions.skipChildren();
1542+
return;
1543+
}
1544+
1545+
visitedBundles.add(descendant);
1546+
1547+
if (
1548+
descendant.type !== bundle.type ||
1549+
fromEnvironmentId(descendant.env).context !==
1550+
fromEnvironmentId(bundle.env).context
1551+
) {
1552+
// Don't skip children - they might be the right type!
1553+
return;
1554+
}
1555+
1556+
// Check ALL assets at once in this descendant bundle
1557+
for (let [asset, dependencies] of assetDependenciesMap) {
1558+
// Skip if already marked as referenced
1559+
if (referencedAssets.has(asset)) {
1560+
continue;
1561+
}
1562+
1563+
// Check if this descendant bundle references the asset
1564+
if (
1565+
!this.bundleHasAsset(descendant, asset) &&
1566+
dependencies.some((dependency) =>
1567+
this.bundleHasDependency(descendant, dependency),
1568+
)
1569+
) {
1570+
referencedAssets.add(asset);
1571+
}
1572+
}
1573+
1574+
// If all assets from assetDependenciesMap are now marked as referenced, we can stop early
1575+
if (allAssetsReferenced()) {
1576+
actions.stop();
1577+
return;
1578+
}
1579+
}, referencer);
1580+
1581+
// If all assets from assetDependenciesMap are referenced, no need to check more sibling bundles
1582+
if (allAssetsReferenced()) {
1583+
break;
1584+
}
1585+
}
1586+
1587+
return referencedAssets;
1588+
}
1589+
14381590
hasParentBundleOfType(bundle: Bundle, type: string): boolean {
14391591
let parents = this.getParentBundles(bundle);
14401592
return (

packages/core/core/src/public/BundleGraph.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,27 @@ export default class BundleGraph<TBundle extends IBundle>
198198
);
199199
}
200200

201+
isAssetReferencedFastCheck(bundle: IBundle, asset: IAsset): boolean | null {
202+
return this.#graph.isAssetReferencedFastCheck(
203+
bundleToInternalBundle(bundle),
204+
assetToAssetValue(asset),
205+
);
206+
}
207+
208+
getReferencedAssets(bundle: IBundle): Set<IAsset> {
209+
let internalReferencedAssets = this.#graph.getReferencedAssets(
210+
bundleToInternalBundle(bundle),
211+
);
212+
213+
// Convert internal assets to public assets
214+
let publicReferencedAssets = new Set<IAsset>();
215+
for (let internalAsset of internalReferencedAssets) {
216+
publicReferencedAssets.add(assetFromValue(internalAsset, this.#options));
217+
}
218+
219+
return publicReferencedAssets;
220+
}
221+
201222
hasParentBundleOfType(bundle: IBundle, type: string): boolean {
202223
return this.#graph.hasParentBundleOfType(
203224
bundleToInternalBundle(bundle),

packages/core/feature-flags/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,16 @@ export const DEFAULT_FEATURE_FLAGS = {
326326
* @since 2025-11-05
327327
*/
328328
nestedPromiseImportFix: process.env.ATLASPACK_BUILD_ENV === 'test',
329+
330+
/**
331+
* Precompute referenced assets in bundles to avoid repeated traversals
332+
* during scope hoisting packaging. This optimization caches which assets
333+
* are referenced in a bundle, reducing O(N*M) calls to O(1).
334+
*
335+
* @author Marcin Szczepanski <[email protected]>
336+
* @since 2025-10-24
337+
*/
338+
precomputeReferencedAssets: process.env.ATLASPACK_BUILD_ENV === 'test',
329339
};
330340

331341
export type FeatureFlags = typeof DEFAULT_FEATURE_FLAGS;

packages/core/types-internal/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,6 +1690,13 @@ export interface BundleGraph<TBundle extends Bundle> {
16901690
isAssetReachableFromBundle(asset: Asset, bundle: Bundle): boolean;
16911691
/** Returns whether an asset is referenced outside the given bundle. */
16921692
isAssetReferenced(bundle: Bundle, asset: Asset): boolean;
1693+
/**
1694+
* Fast checks only for asset reference status. Returns true if fast checks succeed,
1695+
* null if expensive traversal computation is needed.
1696+
*/
1697+
isAssetReferencedFastCheck(bundle: Bundle, asset: Asset): boolean | null;
1698+
/** Returns a set of all assets that are referenced outside the given bundle. */
1699+
getReferencedAssets(bundle: Bundle): Set<Asset>;
16931700
/**
16941701
* Resolves the export `symbol` of `asset` to the source,
16951702
* stopping at the first asset after leaving `bundle`.

packages/packagers/js/src/ScopeHoistingPackager.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ export class ScopeHoistingPackager {
131131
useBothScopeHoistingImprovements: boolean =
132132
getFeatureFlag('applyScopeHoistingImprovementV2') ||
133133
getFeatureFlag('applyScopeHoistingImprovement');
134+
referencedAssetsCache: Map<string, Set<Asset>> = new Map();
134135

135136
constructor(
136137
options: PluginOptions,
@@ -412,6 +413,44 @@ export class ScopeHoistingPackager {
412413
return `$parcel$global.rwr(${params.join(', ')});`;
413414
}
414415

416+
// Helper to check if an asset is referenced, with cache-first + fast-check hybrid approach
417+
isAssetReferencedInBundle(bundle: NamedBundle, asset: Asset): boolean {
418+
// STEP 1: Check expensive computation cache first (fastest when it hits)
419+
if (getFeatureFlag('precomputeReferencedAssets')) {
420+
let bundleId = bundle.id;
421+
let referencedAssets = this.referencedAssetsCache.get(bundleId);
422+
423+
if (referencedAssets) {
424+
// Cache hit - fastest path (~0.001ms)
425+
return referencedAssets.has(asset);
426+
}
427+
}
428+
429+
// STEP 2: Cache miss - try fast checks (~0.01ms)
430+
let fastCheckResult = this.bundleGraph.isAssetReferencedFastCheck(
431+
bundle,
432+
asset,
433+
);
434+
435+
if (fastCheckResult === true) {
436+
// Fast check succeeded - asset is referenced
437+
return true;
438+
}
439+
440+
// STEP 3: Need expensive computation (~20-2000ms)
441+
if (getFeatureFlag('precomputeReferencedAssets')) {
442+
// Compute and cache expensive results for this bundle
443+
let bundleId = bundle.id;
444+
let referencedAssets = this.bundleGraph.getReferencedAssets(bundle);
445+
this.referencedAssetsCache.set(bundleId, referencedAssets);
446+
447+
return referencedAssets.has(asset);
448+
} else {
449+
// No caching - fall back to original expensive method
450+
return this.bundleGraph.isAssetReferenced(bundle, asset);
451+
}
452+
}
453+
415454
async loadAssets() {
416455
type QueueItem = [Asset, {code: string; map: Buffer | undefined | null}];
417456
let queue = new PromiseQueue<QueueItem>({
@@ -431,7 +470,7 @@ export class ScopeHoistingPackager {
431470
if (
432471
asset.meta.shouldWrap ||
433472
this.bundle.env.sourceType === 'script' ||
434-
this.bundleGraph.isAssetReferenced(this.bundle, asset) ||
473+
this.isAssetReferencedInBundle(this.bundle, asset) ||
435474
this.bundleGraph
436475
.getIncomingDependencies(asset)
437476
.some((dep) => dep.meta.shouldWrap && dep.specifierType !== 'url')
@@ -993,7 +1032,7 @@ ${code}
9931032
referencedBundle &&
9941033
referencedBundle.getMainEntry() === resolved &&
9951034
referencedBundle.type === 'js' &&
996-
!this.bundleGraph.isAssetReferenced(referencedBundle, resolved)
1035+
!this.isAssetReferencedInBundle(referencedBundle, resolved)
9971036
) {
9981037
this.addExternal(dep, replacements, referencedBundle);
9991038
this.externalAssets.add(resolved);
@@ -1792,7 +1831,7 @@ ${code}
17921831
return (
17931832
asset.sideEffects === false &&
17941833
nullthrows(this.bundleGraph.getUsedSymbols(asset)).size == 0 &&
1795-
!this.bundleGraph.isAssetReferenced(this.bundle, asset)
1834+
!this.isAssetReferencedInBundle(this.bundle, asset)
17961835
);
17971836
}
17981837

0 commit comments

Comments
 (0)