Optimize bundler performance (#9266)

packages/core/core/src/BundleGraph.js

@@ -174,8 +174,8 @@ export default class BundleGraph {
         : null;
     invariant(assetGraphRootNode != null && assetGraphRootNode.type === 'root');
 
-    for (let [nodeId, node] of assetGraph.nodes) {
-      if (node.type === 'asset') {
+    for (let [nodeId, node] of assetGraph.nodes.entries()) {
+      if (node != null && node.type === 'asset') {
         let {id: assetId} = node.value;
         // Generate a new, short public id for this asset to use.
         // If one already exists, use it.
@@ -187,7 +187,7 @@ export default class BundleGraph {
           publicIdByAssetId.set(assetId, publicId);
           assetPublicIds.add(publicId);
         }
-      } else if (node.type === 'asset_group') {
+      } else if (node != null && node.type === 'asset_group') {
         assetGroupIds.set(nodeId, assetGraph.getNodeIdsConnectedFrom(nodeId));
       }
     }
@@ -2011,7 +2011,8 @@ export default class BundleGraph {
 
   merge(other: BundleGraph) {
     let otherGraphIdToThisNodeId = new Map<NodeId, NodeId>();
-    for (let [otherNodeId, otherNode] of other._graph.nodes) {
+    for (let [otherNodeId, otherNode] of other._graph.nodes.entries()) {
+      if (!otherNode) continue;
       if (this._graph.hasContentKey(otherNode.id)) {
         let existingNodeId = this._graph.getNodeIdByContentKey(otherNode.id);
         otherGraphIdToThisNodeId.set(otherNodeId, existingNodeId);

packages/core/core/src/RequestTracker.js

@@ -724,8 +724,8 @@ export class RequestGraph extends ContentGraph<
       // this means the project root was moved and we need to
       // re-run all requests.
       if (type === 'create' && filePath === '') {
-        for (let [id, node] of this.nodes) {
-          if (node.type === 'request') {
+        for (let [id, node] of this.nodes.entries()) {
+          if (node?.type === 'request') {
             this.invalidNodeIds.add(id);
           }
         }
@@ -1087,8 +1087,8 @@ export default class RequestTracker {
     }
 
     let promises = [];
-    for (let [, node] of this.graph.nodes) {
-      if (node.type !== 'request') {
+    for (let node of this.graph.nodes) {
+      if (!node || node.type !== 'request') {
         continue;
       }
 

packages/core/core/src/SymbolPropagation.js

@@ -629,7 +629,7 @@ function propagateSymbolsUp(
   let runFullPass =
     // If there are n nodes in the graph, then the asset count is approximately
     // n/6 (for every asset, there are ~4 dependencies and ~1 asset_group).
-    assetGraph.nodes.size * (1 / 6) * 0.5 <
+    assetGraph.nodes.length * (1 / 6) * 0.5 <
     changedDepsUsedSymbolsUpDirtyDownAssets.size;
 
   let dirtyDeps;

packages/core/core/src/dumpGraphToGraphViz.js

@@ -60,7 +60,9 @@ export default async function dumpGraphToGraphViz(
   const graphviz = require('graphviz');
   const tempy = require('tempy');
   let g = graphviz.digraph('G');
-  for (let [id, node] of graph.nodes) {
+  // $FlowFixMe
+  for (let [id, node] of graph.nodes.entries()) {
+    if (node == null) continue;
     let n = g.addNode(nodeId(id));
     // $FlowFixMe default is fine. Not every type needs to be in the map.
     n.set('color', COLORS[node.type || 'default']);

packages/core/core/src/public/Asset.js

@@ -25,7 +25,7 @@ import type {Asset as AssetValue, ParcelOptions} from '../types';
 
 import nullthrows from 'nullthrows';
 import Environment from './Environment';
-import Dependency from './Dependency';
+import {getPublicDependency} from './Dependency';
 import {AssetSymbols, MutableAssetSymbols} from './Symbols';
 import UncommittedAsset from '../UncommittedAsset';
 import CommittedAsset from '../CommittedAsset';
@@ -162,7 +162,7 @@ class BaseAsset {
   getDependencies(): $ReadOnlyArray<IDependency> {
     return this.#asset
       .getDependencies()
-      .map(dep => new Dependency(dep, this.#asset.options));
+      .map(dep => getPublicDependency(dep, this.#asset.options));
   }
 
   getCode(): Promise<string> {
@@ -192,6 +192,7 @@ class BaseAsset {
 
 export class Asset extends BaseAsset implements IAsset {
   #asset /*: CommittedAsset | UncommittedAsset */;
+  #env /*: ?Environment */;
 
   constructor(asset: CommittedAsset | UncommittedAsset): Asset {
     let assetValueToAsset = asset.value.committed
@@ -208,6 +209,11 @@ export class Asset extends BaseAsset implements IAsset {
     return this;
   }
 
+  get env(): IEnvironment {
+    this.#env ??= new Environment(this.#asset.value.env, this.#asset.options);
+    return this.#env;
+  }
+
   get stats(): Stats {
     return this.#asset.value.stats;
   }

packages/core/core/src/public/Bundle.js

@@ -27,7 +27,10 @@ import {DefaultWeakMap} from '@parcel/utils';
 import {assetToAssetValue, assetFromValue} from './Asset';
 import {mapVisitor} from '@parcel/graph';
 import Environment from './Environment';
-import Dependency, {dependencyToInternalDependency} from './Dependency';
+import {
+  dependencyToInternalDependency,
+  getPublicDependency,
+} from './Dependency';
 import Target from './Target';
 import {BundleBehaviorNames} from '../types';
 import {fromProjectPath} from '../projectPath';
@@ -179,7 +182,7 @@ export class Bundle implements IBundle {
         } else if (node.type === 'dependency') {
           return {
             type: 'dependency',
-            value: new Dependency(node.value, this.#options),
+            value: getPublicDependency(node.value, this.#options),
           };
         }
       }, visit),

packages/core/core/src/public/BundleGraph.js

@@ -23,7 +23,10 @@ import nullthrows from 'nullthrows';
 import {mapVisitor} from '@parcel/graph';
 import {assetFromValue, assetToAssetValue, Asset} from './Asset';
 import {bundleToInternalBundle} from './Bundle';
-import Dependency, {dependencyToInternalDependency} from './Dependency';
+import Dependency, {
+  dependencyToInternalDependency,
+  getPublicDependency,
+} from './Dependency';
 import {targetToInternalTarget} from './Target';
 import {fromInternalSourceLocation} from '../utils';
 import BundleGroup, {bundleGroupToInternalBundleGroup} from './BundleGroup';
@@ -90,7 +93,7 @@ export default class BundleGraph<TBundle: IBundle>
   getIncomingDependencies(asset: IAsset): Array<IDependency> {
     return this.#graph
       .getIncomingDependencies(assetToAssetValue(asset))
-      .map(dep => new Dependency(dep, this.#options));
+      .map(dep => getPublicDependency(dep, this.#options));
   }
 
   getAssetWithDependency(dep: IDependency): ?IAsset {
@@ -158,7 +161,7 @@ export default class BundleGraph<TBundle: IBundle>
   getDependencies(asset: IAsset): Array<IDependency> {
     return this.#graph
       .getDependencies(assetToAssetValue(asset))
-      .map(dep => new Dependency(dep, this.#options));
+      .map(dep => getPublicDependency(dep, this.#options));
   }
 
   isAssetReachableFromBundle(asset: IAsset, bundle: IBundle): boolean {
@@ -256,18 +259,27 @@ export default class BundleGraph<TBundle: IBundle>
   traverse<TContext>(
     visit: GraphVisitor<BundleGraphTraversable, TContext>,
     start?: ?IAsset,
+    opts?: ?{|skipUnusedDependencies?: boolean|},
   ): ?TContext {
     return this.#graph.traverse(
-      mapVisitor(
-        node =>
-          node.type === 'asset'
-            ? {type: 'asset', value: assetFromValue(node.value, this.#options)}
-            : {
-                type: 'dependency',
-                value: new Dependency(node.value, this.#options),
-              },
-        visit,
-      ),
+      mapVisitor((node, actions) => {
+        // Skipping unused dependencies here is faster than doing an isDependencySkipped check inside the visitor
+        // because the node needs to be re-looked up by id from the hashmap.
+        if (
+          opts?.skipUnusedDependencies &&
+          node.type === 'dependency' &&
+          (node.hasDeferred || node.excluded)
+        ) {
+          actions.skipChildren();
+          return null;
+        }
+        return node.type === 'asset'
+          ? {type: 'asset', value: assetFromValue(node.value, this.#options)}
+          : {
+              type: 'dependency',
+              value: getPublicDependency(node.value, this.#options),
+            };
+      }, visit),
       start ? assetToAssetValue(start) : undefined,
     );
   }

packages/core/core/src/public/Dependency.js

@@ -42,16 +42,23 @@ export function dependencyToInternalDependency(
   return nullthrows(_dependencyToInternalDependency.get(dependency));
 }
 
+export function getPublicDependency(
+  dep: InternalDependency,
+  options: ParcelOptions,
+): Dependency {
+  let existing = internalDependencyToDependency.get(dep);
+  if (existing != null) {
+    return existing;
+  }
+
+  return new Dependency(dep, options);
+}
+
 export default class Dependency implements IDependency {
   #dep /*: InternalDependency */;
   #options /*: ParcelOptions */;
 
   constructor(dep: InternalDependency, options: ParcelOptions): Dependency {
-    let existing = internalDependencyToDependency.get(dep);
-    if (existing != null) {
-      return existing;
-    }
-
     this.#dep = dep;
     this.#options = options;
     _dependencyToInternalDependency.set(this, dep);

packages/core/core/src/requests/AssetGraphRequest.js

@@ -242,7 +242,7 @@ export class AssetGraphBuilder {
       throw errors[0];
     }
 
-    if (this.assetGraph.nodes.size > 1) {
+    if (this.assetGraph.nodes.length > 1) {
       await dumpGraphToGraphViz(
         this.assetGraph,
         'AssetGraph_' + this.name + '_before_prop',

packages/core/core/src/requests/PathRequest.js

@@ -26,7 +26,7 @@ import nullthrows from 'nullthrows';
 import path from 'path';
 import {normalizePath} from '@parcel/utils';
 import {report} from '../ReporterRunner';
-import PublicDependency from '../public/Dependency';
+import {getPublicDependency} from '../public/Dependency';
 import PluginOptions from '../public/PluginOptions';
 import ParcelConfig from '../ParcelConfig';
 import createParcelConfigRequest, {
@@ -240,7 +240,7 @@ export class ResolverRunner {
   }
 
   async resolve(dependency: Dependency): Promise<ResolverResult> {
-    let dep = new PublicDependency(dependency, this.options);
+    let dep = getPublicDependency(dependency, this.options);
     report({
       type: 'buildProgress',
       phase: 'resolving',

packages/core/core/test/AssetGraph.test.js

@@ -260,7 +260,7 @@ describe('AssetGraph', () => {
       nodeFromAssetGroup(req).id,
     );
     let dependencyNodeId = graph.getNodeIdByContentKey(dep.id);
-    assert(graph.nodes.has(assetGroupNodeId));
+    assert(graph.hasNode(assetGroupNodeId));
     assert(graph.hasEdge(dependencyNodeId, assetGroupNodeId));
 
     let req2 = {
@@ -272,13 +272,13 @@ describe('AssetGraph', () => {
     let assetGroupNodeId2 = graph.getNodeIdByContentKey(
       nodeFromAssetGroup(req2).id,
     );
-    assert(!graph.nodes.has(assetGroupNodeId));
-    assert(graph.nodes.has(assetGroupNodeId2));
+    assert(!graph.hasNode(assetGroupNodeId));
+    assert(graph.hasNode(assetGroupNodeId2));
     assert(graph.hasEdge(dependencyNodeId, assetGroupNodeId2));
     assert(!graph.hasEdge(dependencyNodeId, assetGroupNodeId));
 
     graph.resolveDependency(dep, req2, '5');
-    assert(graph.nodes.has(assetGroupNodeId2));
+    assert(graph.hasNode(assetGroupNodeId2));
     assert(graph.hasEdge(dependencyNodeId, assetGroupNodeId2));
   });
 
@@ -389,11 +389,11 @@ describe('AssetGraph', () => {
       [...assets[1].dependencies.values()][0].id,
     );
 
-    assert(graph.nodes.has(nodeId1));
-    assert(graph.nodes.has(nodeId2));
-    assert(graph.nodes.has(nodeId3));
-    assert(graph.nodes.has(dependencyNodeId1));
-    assert(graph.nodes.has(dependencyNodeId2));
+    assert(graph.hasNode(nodeId1));
+    assert(graph.hasNode(nodeId2));
+    assert(graph.hasNode(nodeId3));
+    assert(graph.hasNode(dependencyNodeId1));
+    assert(graph.hasNode(dependencyNodeId2));
     assert(graph.hasEdge(assetGroupNode, nodeId1));
     assert(graph.hasEdge(assetGroupNode, nodeId2));
     assert(graph.hasEdge(assetGroupNode, nodeId3));
@@ -435,11 +435,11 @@ describe('AssetGraph', () => {
 
     graph.resolveAssetGroup(req, assets2, '5');
 
-    assert(graph.nodes.has(nodeId1));
-    assert(graph.nodes.has(nodeId2));
-    assert(!graph.nodes.has(nodeId3));
-    assert(graph.nodes.has(dependencyNodeId1));
-    assert(!graph.nodes.has(dependencyNodeId2));
+    assert(graph.hasNode(nodeId1));
+    assert(graph.hasNode(nodeId2));
+    assert(!graph.hasNode(nodeId3));
+    assert(graph.hasNode(dependencyNodeId1));
+    assert(!graph.hasNode(dependencyNodeId2));
     assert(graph.hasEdge(assetGroupNode, nodeId1));
     assert(graph.hasEdge(assetGroupNode, nodeId2));
     assert(!graph.hasEdge(assetGroupNode, nodeId3));

packages/core/core/test/PublicDependency.test.js

@@ -3,7 +3,7 @@
 import assert from 'assert';
 import {createEnvironment} from '../src/Environment';
 import {createDependency} from '../src/Dependency';
-import Dependency from '../src/public/Dependency';
+import {getPublicDependency} from '../src/public/Dependency';
 import {DEFAULT_OPTIONS} from './test-utils';
 
 describe('Public Dependency', () => {
@@ -15,8 +15,8 @@ describe('Public Dependency', () => {
     });
 
     assert.equal(
-      new Dependency(internalDependency, DEFAULT_OPTIONS),
-      new Dependency(internalDependency, DEFAULT_OPTIONS),
+      getPublicDependency(internalDependency, DEFAULT_OPTIONS),
+      getPublicDependency(internalDependency, DEFAULT_OPTIONS),
     );
   });
 });

packages/core/core/test/SymbolPropagation.test.js

@@ -197,7 +197,7 @@ function assertUsedSymbols(
   if (isLibrary) {
     let entryDep = nullthrows(
       [...graph.nodes.values()].find(
-        n => n.type === 'dependency' && n.value.sourceAssetId == null,
+        n => n?.type === 'dependency' && n.value.sourceAssetId == null,
       ),
     );
     invariant(entryDep.type === 'dependency');
@@ -240,12 +240,12 @@ function assertUsedSymbols(
     }
   }
 
-  for (let [nodeId, node] of graph.nodes) {
-    if (node.type === 'asset') {
+  for (let [nodeId, node] of graph.nodes.entries()) {
+    if (node?.type === 'asset') {
       let filePath = fromProjectPathUnix(node.value.filePath);
       let expected = new Set(nullthrows(expectedAsset.get(filePath)));
       assertSetEqual(node.usedSymbols, expected, filePath);
-    } else if (node.type === 'dependency' && node.value.sourcePath != null) {
+    } else if (node?.type === 'dependency' && node.value.sourcePath != null) {
       let resolutionId = graph.getNodeIdsConnectedFrom(nodeId)[0];
       let resolution = nullthrows(graph.getNode(resolutionId));
       invariant(resolution.type === 'asset_group');
@@ -373,14 +373,14 @@ function changeDependency(
 ): Iterable<[ContentKey, Asset]> {
   let sourceAssetNode = nullthrowsAssetNode(
     [...graph.nodes.values()].find(
-      n => n.type === 'asset' && n.value.filePath === from,
+      n => n?.type === 'asset' && n.value.filePath === from,
     ),
   );
   sourceAssetNode.usedSymbolsDownDirty = true;
   let depNode = nullthrowsDependencyNode(
     [...graph.nodes.values()].find(
       n =>
-        n.type === 'dependency' &&
+        n?.type === 'dependency' &&
         n.value.sourcePath === from &&
         n.value.specifier === to,
     ),
@@ -396,7 +396,7 @@ function changeAsset(
 ): Iterable<[ContentKey, Asset]> {
   let node = nullthrowsAssetNode(
     [...graph.nodes.values()].find(
-      n => n.type === 'asset' && n.value.filePath === asset,
+      n => n?.type === 'asset' && n.value.filePath === asset,
     ),
   );
   node.usedSymbolsUpDirty = true;

packages/core/graph/package.json

@@ -20,6 +20,7 @@
     "node": ">= 12.0.0"
   },
   "dependencies": {
+    "@parcel/utils": "^2.9.3",
     "nullthrows": "^1.1.1"
   }
 }