Skip to content

Commit

Permalink
Optimize BundleGraph::getAssetWithDependency
Browse files Browse the repository at this point in the history
Prevents creating temporary arrays when finding the asset linked to a
dependency. This is a relatively hot path in the bundler and can
improve its performance.
  • Loading branch information
yamadapc committed Dec 3, 2024
1 parent 3701139 commit 9ba072a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 9 deletions.
21 changes: 15 additions & 6 deletions packages/core/core/src/BundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -1773,14 +1773,23 @@ export default class BundleGraph {
return null;
}

let res = this._graph.getNodeIdsConnectedTo(
let res = null;
let count = 0;
this._graph.forEachNodeIdConnectedTo(
this._graph.getNodeIdByContentKey(dep.id),
(node) => {
res = node;
count += 1;
if (count > 1) {
throw new Error(
'Expected a single asset to be connected to a dependency',
);
}
},
);
invariant(
res.length <= 1,
'Expected a single asset to be connected to a dependency',
);
let resNode = this._graph.getNode(res[0]);

invariant(res != null, 'Expected an asset to be connected to a dependency');
let resNode = this._graph.getNode(res);
if (resNode?.type === 'asset') {
return resNode.value;
}
Expand Down
6 changes: 4 additions & 2 deletions packages/core/graph/src/AdjacencyList.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,13 +619,15 @@ export default class AdjacencyList<TEdgeType: number = 1> {
}
}

forEachNodeIdConnectedTo(to: NodeId, fn: (nodeId: NodeId) => void) {
forEachNodeIdConnectedTo(to: NodeId, fn: (nodeId: NodeId) => boolean | void) {
let node = this.#nodes.head(to);
while (node !== null) {
let edge = this.#nodes.firstIn(node);
while (edge !== null) {
let from = this.#edges.from(edge);
fn(from);
if (fn(from)) {
return;
}
edge = this.#edges.nextIn(edge);
}
node = this.#nodes.next(node);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/graph/src/Graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export default class Graph<TNode, TEdgeType: number = 1> {
return this.adjacencyList.hasEdge(from, to, type);
}

forEachNodeIdConnectedTo(to: NodeId, fn: (nodeId: NodeId) => void) {
forEachNodeIdConnectedTo(to: NodeId, fn: (nodeId: NodeId) => boolean | void) {
this._assertHasNodeId(to);

this.adjacencyList.forEachNodeIdConnectedTo(to, fn);
Expand Down
20 changes: 20 additions & 0 deletions packages/core/graph/test/AdjacencyList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,26 @@ describe('AdjacencyList', () => {
assert.deepEqual(nodeIds, [a, c]);
});

it('stops traversal if the return value is true', () => {
const graph = new AdjacencyList();
const a = graph.addNode();
const b = graph.addNode();
const c = graph.addNode();
const d = graph.addNode();

graph.addEdge(a, d);
graph.addEdge(b, d);
graph.addEdge(c, d);

const nodeIds = [];
graph.forEachNodeIdConnectedTo(d, (nodeId) => {
nodeIds.push(nodeId);
return true;
});

assert.deepEqual(nodeIds, [a]);
});

it('terminates if the graph is cyclic', () => {
const graph = new AdjacencyList();
const a = graph.addNode();
Expand Down

0 comments on commit 9ba072a

Please sign in to comment.