Skip to content

Commit d241552

Browse files
authored
[turbopack] Rename graph traversal algorithms (#87119)
* remove `traverse_edges_from_entry_dfs` it is dead * rename a number of traversal methods to shorter names. mostly by dropping `from_entries` which i don't believe is useful. * rename `travserve_edges_from_entries_dfs_reversed` to `traverse_edges_reverse_dfs` which is a bit better, but could probably still use some work. I explored ways to share logic across our DFS implementations and ultimately determined that the juice wasn't worth the squeeze
1 parent 73dea86 commit d241552

File tree

11 files changed

+43
-93
lines changed

11 files changed

+43
-93
lines changed

crates/next-api/src/analyze.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ pub async fn analyze_module_graphs(module_graphs: Vc<ModuleGraphs>) -> Result<Vc
424424
let mut all_async_edges = FxIndexSet::default();
425425
for &module_graph in module_graphs.await? {
426426
let module_graph = module_graph.read_graphs().await?;
427-
module_graph.traverse_all_edges_unordered(|parent, node| {
427+
module_graph.traverse_edges_unordered(|parent, node| {
428428
if let Some((parent_node, reference)) = parent {
429429
all_modules.insert(parent_node);
430430
all_modules.insert(node);

crates/next-api/src/module_graph.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ impl NextDynamicGraph {
173173

174174
// module -> the client reference entry (if any)
175175
let mut state_map = FxHashMap::default();
176-
graph.traverse_edges_from_entries_dfs(
176+
graph.traverse_edges_dfs(
177177
entries,
178178
&mut (),
179179
|parent_info, node, _| {
@@ -351,7 +351,7 @@ impl ServerActionsGraph {
351351
}
352352

353353
let mut result = FxIndexMap::default();
354-
graph.traverse_nodes_from_entries_dfs(
354+
graph.traverse_nodes_dfs(
355355
vec![entry],
356356
&mut result,
357357
|node, result| {
@@ -559,7 +559,7 @@ impl ClientReferencesGraph {
559559
let mut server_components = FxIndexSet::default();
560560

561561
// Perform a DFS traversal to find all server components included by this page.
562-
graph.traverse_nodes_from_entries_dfs(
562+
graph.traverse_nodes_dfs(
563563
entries,
564564
&mut (),
565565
|node, _| {
@@ -615,7 +615,7 @@ impl ClientReferencesGraph {
615615
// necessarily rendered at the same time (not-found, or parallel routes), we need to
616616
// determine the order of client references individually for each server component.
617617
for sc in server_components.iter().copied() {
618-
graph.traverse_nodes_from_entries_dfs(
618+
graph.traverse_nodes_dfs(
619619
std::iter::once(ResolvedVc::upcast(sc)),
620620
&mut (),
621621
|node, _| {
@@ -786,7 +786,7 @@ async fn validate_pages_css_imports_individual(
786786

787787
let mut candidates = vec![];
788788

789-
graph.traverse_edges_from_entries_dfs(
789+
graph.traverse_edges_dfs(
790790
entries,
791791
&mut (),
792792
|parent_info, node, _| {

crates/next-api/src/routes_hashes_manifest.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ pub async fn endpoint_hashes(
6464

6565
let module_graph = module_graph.read_graphs().await?;
6666

67-
module_graph.traverse_nodes_from_entries_dfs(
67+
module_graph.traverse_nodes_dfs(
6868
modules,
6969
&mut all_modules,
7070
|module, all_modules| {

crates/next-api/src/webpack_stats.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ where
5858
let asset_reasons = {
5959
let module_graph = module_graph.read_graphs().await?;
6060
let mut edges = vec![];
61-
module_graph.traverse_all_edges_unordered(|parent, current| {
61+
module_graph.traverse_edges_unordered(|parent, current| {
6262
if let Some((parent_node, r)) = parent {
6363
edges.push((
6464
parent_node,

turbopack/crates/turbopack-core/src/module_graph/async_module_info.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ async fn compute_async_module_info_single(
8080
let mut async_modules = FxHashSet::default();
8181
async_modules.extend(self_async_modules.iter());
8282

83-
graph.traverse_edges_from_entries_dfs_reversed(
83+
graph.traverse_edges_reverse_dfs(
8484
self_async_modules,
8585
&mut (),
8686
// child is the previously visited module which must be async

turbopack/crates/turbopack-core/src/module_graph/chunk_group_info.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ pub async fn compute_chunk_group_info(graph: &ModuleGraphRef) -> Result<Vc<Chunk
411411
let module_depth: FxHashMap<ResolvedVc<Box<dyn Module>>, usize> = {
412412
let mut module_depth =
413413
FxHashMap::with_capacity_and_hasher(module_count, Default::default());
414-
graph.traverse_edges_from_entries_bfs(
414+
graph.traverse_edges_bfs(
415415
entries.iter().flat_map(|e| e.entries()),
416416
|parent, node| {
417417
if let Some((parent, _)) = parent {

turbopack/crates/turbopack-core/src/module_graph/merged_modules.rs

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,21 +93,18 @@ pub async fn compute_merged_modules(module_graph: Vc<ModuleGraph>) -> Result<Vc<
9393

9494
let mut module_depth =
9595
FxHashMap::with_capacity_and_hasher(module_count, Default::default());
96-
module_graph.traverse_edges_from_entries_bfs(
97-
entries.iter().copied(),
98-
|parent, node| {
99-
if let Some((parent, _)) = parent {
100-
let parent_depth = *module_depth
101-
.get(&parent)
102-
.context("Module depth not found")?;
103-
module_depth.entry(node).or_insert(parent_depth + 1);
104-
} else {
105-
module_depth.insert(node, 0);
106-
};
96+
module_graph.traverse_edges_bfs(entries.iter().copied(), |parent, node| {
97+
if let Some((parent, _)) = parent {
98+
let parent_depth = *module_depth
99+
.get(&parent)
100+
.context("Module depth not found")?;
101+
module_depth.entry(node).or_insert(parent_depth + 1);
102+
} else {
103+
module_depth.insert(node, 0);
104+
};
107105

108-
Ok(GraphTraversalAction::Continue)
109-
},
110-
)?;
106+
Ok(GraphTraversalAction::Continue)
107+
})?;
111108
module_depth
112109
};
113110

@@ -326,7 +323,7 @@ pub async fn compute_merged_modules(module_graph: Vc<ModuleGraph>) -> Result<Vc<
326323
// leading to the list `a, b` which is not execution order.
327324
let mut visited = FxHashSet::default();
328325

329-
module_graph.traverse_edges_from_entries_dfs(
326+
module_graph.traverse_edges_dfs(
330327
chunk_group.entries(),
331328
&mut (),
332329
|parent_info, node, _| {
@@ -468,7 +465,7 @@ pub async fn compute_merged_modules(module_graph: Vc<ModuleGraph>) -> Result<Vc<
468465
let mut exposed_modules_namespace: FxHashSet<ResolvedVc<Box<dyn Module>>> =
469466
FxHashSet::with_capacity_and_hasher(module_merged_groups.len(), Default::default());
470467

471-
module_graph.traverse_edges_from_entries_dfs(
468+
module_graph.traverse_edges_dfs(
472469
entries,
473470
&mut (),
474471
|_, _, _| Ok(GraphTraversalAction::Continue),

turbopack/crates/turbopack-core/src/module_graph/mod.rs

Lines changed: 16 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,7 +1107,7 @@ impl ModuleGraphRef {
11071107
/// - Receives the module and the `state`
11081108
/// - Can return [GraphTraversalAction]s to control the traversal
11091109
/// * `visit_postorder` - Called after visiting children of a node.
1110-
pub fn traverse_nodes_from_entries_dfs<S>(
1110+
pub fn traverse_nodes_dfs<S>(
11111111
&self,
11121112
entries: impl IntoIterator<Item = ResolvedVc<Box<dyn Module>>>,
11131113
state: &mut S,
@@ -1168,7 +1168,7 @@ impl ModuleGraphRef {
11681168
/// - Receives (originating &SingleModuleGraphNode, edge &ChunkingType), target
11691169
/// &SingleModuleGraphNode, state &S
11701170
/// - Can return [GraphTraversalAction]s to control the traversal
1171-
pub fn traverse_edges_from_entries_bfs(
1171+
pub fn traverse_edges_bfs(
11721172
&self,
11731173
entries: impl IntoIterator<Item = ResolvedVc<Box<dyn Module>>>,
11741174
mut visitor: impl FnMut(
@@ -1209,55 +1209,6 @@ impl ModuleGraphRef {
12091209
Ok(())
12101210
}
12111211

1212-
/// Traverses all reachable edges exactly once and calls the visitor with the edge source and
1213-
/// target.
1214-
///
1215-
/// This means that target nodes can be revisited (once per incoming edge).
1216-
///
1217-
/// * `entry` - The entry module to start the traversal from
1218-
/// * `visitor` - Called before visiting the children of a node.
1219-
/// - Receives (originating &SingleModuleGraphNode, edge &ChunkingType), target
1220-
/// &SingleModuleGraphNode, state &S
1221-
/// - Can return [GraphTraversalAction]s to control the traversal
1222-
pub fn traverse_edges_from_entry_dfs(
1223-
&self,
1224-
entries: impl IntoIterator<Item = ResolvedVc<Box<dyn Module>>>,
1225-
mut visitor: impl FnMut(
1226-
Option<(ResolvedVc<Box<dyn Module>>, &'_ RefData)>,
1227-
ResolvedVc<Box<dyn Module>>,
1228-
) -> GraphTraversalAction,
1229-
) -> Result<()> {
1230-
let mut stack = entries
1231-
.into_iter()
1232-
.map(|e| self.get_entry(e))
1233-
.collect::<Result<Vec<_>>>()?;
1234-
let mut visited = FxHashSet::default();
1235-
for entry_node in &stack {
1236-
visitor(None, self.get_node(*entry_node)?.module());
1237-
}
1238-
while let Some(node) = stack.pop() {
1239-
if visited.insert(node) {
1240-
let node_weight = self.get_node(node)?;
1241-
for (edge, succ) in self.iter_graphs_neighbors_rev(node, Direction::Outgoing) {
1242-
let succ_weight = self.get_node(succ)?;
1243-
let action = visitor(
1244-
Some((node_weight.module(), self.get_edge(edge)?)),
1245-
succ_weight.module(),
1246-
);
1247-
if !self.should_visit_node(succ_weight, Direction::Outgoing) {
1248-
continue;
1249-
}
1250-
let succ = succ_weight.target_idx(Direction::Outgoing).unwrap_or(succ);
1251-
if !visited.contains(&succ) && action == GraphTraversalAction::Continue {
1252-
stack.push(succ);
1253-
}
1254-
}
1255-
}
1256-
}
1257-
1258-
Ok(())
1259-
}
1260-
12611212
/// Traverses all edges exactly once (in an unspecified order) and calls the visitor with the
12621213
/// edge source and target.
12631214
///
@@ -1266,7 +1217,7 @@ impl ModuleGraphRef {
12661217
/// * `visitor` - Called before visiting the children of a node.
12671218
/// - Receives (originating &SingleModuleGraphNode, edge &ChunkingType), target
12681219
/// &SingleModuleGraphNode
1269-
pub fn traverse_all_edges_unordered(
1220+
pub fn traverse_edges_unordered(
12701221
&self,
12711222
mut visitor: impl FnMut(
12721223
Option<(ResolvedVc<Box<dyn Module>>, &'_ RefData)>,
@@ -1275,7 +1226,9 @@ impl ModuleGraphRef {
12751226
) -> Result<()> {
12761227
let entries = self.graphs.iter().flat_map(|g| g.entry_modules());
12771228

1278-
self.traverse_edges_from_entries_dfs(
1229+
// Despite the name we need to do a DFS to respect 'reachability' if an edge was trimmed we
1230+
// should not follow it, and this is a reasonable way to do that.
1231+
self.traverse_edges_dfs(
12791232
entries,
12801233
&mut (),
12811234
|parent, target, _| {
@@ -1304,7 +1257,7 @@ impl ModuleGraphRef {
13041257
/// * `visit_postorder` - Called after visiting the children of a node. Return
13051258
/// - Receives: (originating &SingleModuleGraphNode, edge &ChunkingType), target
13061259
/// &SingleModuleGraphNode, state &S
1307-
pub fn traverse_edges_from_entries_dfs<S>(
1260+
pub fn traverse_edges_dfs<S>(
13081261
&self,
13091262
entries: impl IntoIterator<Item = ResolvedVc<Box<dyn Module>>>,
13101263
state: &mut S,
@@ -1319,7 +1272,7 @@ impl ModuleGraphRef {
13191272
&mut S,
13201273
) -> Result<()>,
13211274
) -> Result<()> {
1322-
self.traverse_edges_from_entries_dfs_impl::<S>(
1275+
self.traverse_edges_dfs_impl::<S>(
13231276
entries,
13241277
state,
13251278
visit_preorder,
@@ -1344,7 +1297,7 @@ impl ModuleGraphRef {
13441297
/// * `visit_postorder` - Called after visiting the parents of a node. Return
13451298
/// - Receives: (originating &SingleModuleGraphNode, edge &ChunkingType), target
13461299
/// &SingleModuleGraphNode, state &S
1347-
pub fn traverse_edges_from_entries_dfs_reversed<S>(
1300+
pub fn traverse_edges_reverse_dfs<S>(
13481301
&self,
13491302
entries: impl IntoIterator<Item = ResolvedVc<Box<dyn Module>>>,
13501303
state: &mut S,
@@ -1359,7 +1312,7 @@ impl ModuleGraphRef {
13591312
&mut S,
13601313
) -> Result<()>,
13611314
) -> Result<()> {
1362-
self.traverse_edges_from_entries_dfs_impl::<S>(
1315+
self.traverse_edges_dfs_impl::<S>(
13631316
entries,
13641317
state,
13651318
visit_preorder,
@@ -1368,7 +1321,7 @@ impl ModuleGraphRef {
13681321
)
13691322
}
13701323

1371-
fn traverse_edges_from_entries_dfs_impl<S>(
1324+
fn traverse_edges_dfs_impl<S>(
13721325
&self,
13731326
entries: impl IntoIterator<Item = ResolvedVc<Box<dyn Module>>>,
13741327
state: &mut S,
@@ -1972,7 +1925,7 @@ pub mod tests {
19721925
let mut preorder_visits = Vec::new();
19731926
let mut postorder_visits = Vec::new();
19741927

1975-
graph.traverse_edges_from_entries_dfs(
1928+
graph.traverse_edges_dfs(
19761929
entry_modules,
19771930
&mut (),
19781931
|parent, target, _| {
@@ -2032,7 +1985,7 @@ pub mod tests {
20321985
let mut preorder_visits = Vec::new();
20331986
let mut postorder_visits = Vec::new();
20341987

2035-
graph.traverse_edges_from_entries_dfs(
1988+
graph.traverse_edges_dfs(
20361989
entry_modules,
20371990
&mut (),
20381991
|parent, target, _| {
@@ -2186,7 +2139,7 @@ pub mod tests {
21862139
// test traversing forward from a in the child graph
21872140
{
21882141
let mut visited_forward = Vec::new();
2189-
child_graph.traverse_edges_from_entries_dfs(
2142+
child_graph.traverse_edges_dfs(
21902143
vec![a_module],
21912144
&mut (),
21922145
|_parent, child, _state_| {
@@ -2237,7 +2190,7 @@ pub mod tests {
22372190
.next()
22382191
.unwrap();
22392192
let mut visited_reverse = Vec::new();
2240-
child_graph.traverse_edges_from_entries_dfs_reversed(
2193+
child_graph.traverse_edges_reverse_dfs(
22412194
vec![d_module],
22422195
&mut (),
22432196
|_parent, child, _state_| {
@@ -2259,7 +2212,7 @@ pub mod tests {
22592212
// VisitedModule in this graph
22602213
{
22612214
let mut visited_reverse = Vec::new();
2262-
child_graph.traverse_edges_from_entries_dfs_reversed(
2215+
child_graph.traverse_edges_reverse_dfs(
22632216
vec![b_module],
22642217
&mut (),
22652218
|_parent, child, _state_| {

turbopack/crates/turbopack-core/src/module_graph/module_batches.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ impl PreBatches {
289289
this: self,
290290
};
291291
let mut visited = FxHashSet::default();
292-
module_graph.traverse_edges_from_entries_dfs(
292+
module_graph.traverse_edges_dfs(
293293
std::iter::once(entry),
294294
&mut state,
295295
|parent_info, node, state| {
@@ -355,7 +355,7 @@ pub async fn compute_module_batches(
355355

356356
// Walk the module graph and mark all modules that are boundary modules (referenced from a
357357
// different chunk group bitmap)
358-
module_graph.traverse_all_edges_unordered(|parent, node| {
358+
module_graph.traverse_edges_unordered(|parent, node| {
359359
if let Some((parent, ty)) = parent {
360360
let std::collections::hash_set::Entry::Vacant(entry) =
361361
pre_batches.boundary_modules.entry(node)

turbopack/crates/turbopack-core/src/module_graph/side_effect_module_info.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ async fn compute_side_effect_free_module_info_single(
7575
// dependencies are side effect free.
7676

7777
let mut locally_side_effect_free_modules_that_have_side_effects = FxHashSet::default();
78-
graph.traverse_edges_from_entries_dfs_reversed(
78+
graph.traverse_edges_reverse_dfs(
7979
// Start from all the side effectful nodes
8080
module_side_effects.iter().filter_map(|(m, e)| {
8181
if *e == ModuleSideEffects::SideEffectful {

0 commit comments

Comments
 (0)