Skip to content

Commit a51a2e4

Browse files
authored
[turbopack] Support traversing the graph in reverse order (#86427)
This makes certain aggregations trivial since we are are guaranteed to only traverse relevant edges. Use it to fix a bug in the async module identification logic. Previously we would aggregate async cycles after propagating 'asyncness' through the DFS post order traversal, but this could cause us to fail to propagate async to all reverse dependencies depending on which part of a cycle a node would happen to visit first. By traversing starting from the async modules we just need to mark everything we find and not worry about cycles at all since the underlying DFS mechanism will ensure we don't loop. In addition to only requiring a single pass, this is guaranteed to visit fewer nodes in that single pass. One caveat is that reverse traversals only work within a single `SingleModuleGraph` so a debug assert was added to prevent misuse Closes #85988 Closes #86391 Closes PACK-5806
1 parent cf480fc commit a51a2e4

File tree

10 files changed

+390
-65
lines changed

10 files changed

+390
-65
lines changed

contributing/core/testing.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,19 @@ we attempt to capture traces of the playwright run to make debugging the failure
103103
A test-trace artifact should be uploaded after the workflow completes which can be downloaded, unzipped,
104104
and then inspected with `pnpm playwright show-trace ./path/to/trace`
105105

106+
To attach the chrome debugger to next the easiest approach is to modify the `createNext` call in your test to pass `--inspect` to next.
107+
108+
```js
109+
const next = await createNext({
110+
...
111+
startArgs: =['--inspect'],
112+
})
113+
```
114+
115+
Consider also sett `NEXT_E2E_TEST_TIMEOUT=0`
116+
117+
To debug the test process itself you need to pass the `inspect` flag to the node process running jest. e.g. `IS_TURBOPACK_TEST=1 TURBOPACK_DEV=1 NEXT_TEST_MODE=dev node --inspect node_modules/jest/bin/jest.js ...`
118+
106119
### Profiling tests
107120

108121
Add `NEXT_TEST_TRACE=1` to enable test profiling. It's useful for improving our testing infrastructure.

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

Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use anyhow::Result;
22
use rustc_hash::FxHashSet;
3-
use turbo_tasks::{ResolvedVc, TryJoinIterExt, Vc};
3+
use turbo_tasks::{ResolvedVc, TryFlatJoinIterExt, Vc};
44

55
use crate::{
66
module::{Module, Modules},
@@ -57,52 +57,53 @@ async fn compute_async_module_info_single(
5757
let graph = graph.read().await?;
5858

5959
let self_async_modules = graph
60-
.graphs
61-
.first()
62-
.unwrap()
63-
.iter_nodes()
64-
.map(async |node| Ok((node, *node.is_self_async().await?)))
65-
.try_join()
66-
.await?
67-
.into_iter()
68-
.flat_map(|(k, v)| v.then_some(k))
69-
.chain(parent_async_modules.iter().copied())
70-
.collect::<FxHashSet<_>>();
60+
.enumerate_nodes()
61+
.map(async |(_, node)| {
62+
Ok(match node {
63+
super::SingleModuleGraphNode::Module(node) => {
64+
node.is_self_async().await?.then_some(*node)
65+
}
66+
super::SingleModuleGraphNode::VisitedModule { idx: _, module } => {
67+
// If a module is async in the parent then we need to mark reverse dependencies
68+
// async in this graph as well.
69+
parent_async_modules.contains(module).then_some(*module)
70+
}
71+
})
72+
})
73+
.try_flat_join()
74+
.await?;
7175

7276
// To determine which modules are async, we need to propagate the self-async flag to all
73-
// importers, which is done using a postorder traversal of the graph.
74-
//
75-
// This however doesn't cover cycles of async modules, which are handled by determining all
76-
// strongly-connected components, and then marking all the whole SCC as async if one of the
77-
// modules in the SCC is async.
77+
// importers, which is done using a reverse traversal over the graph
78+
// Because we walk edges in the reverse direction we can trivially handle things like cycles
79+
// without actually computing them.
80+
let mut async_modules = FxHashSet::default();
81+
async_modules.extend(self_async_modules.iter());
7882

79-
let mut async_modules = self_async_modules;
80-
graph.traverse_edges_from_entries_dfs(
81-
graph.graphs.first().unwrap().entry_modules(),
83+
graph.traverse_edges_from_entries_dfs_reversed(
84+
self_async_modules,
8285
&mut (),
83-
|_, _, _| Ok(GraphTraversalAction::Continue),
84-
|parent_info, module, _| {
85-
let Some((parent_module, ref_data)) = parent_info else {
86-
// An entry module
87-
return Ok(());
88-
};
89-
90-
if ref_data.chunking_type.is_inherit_async() && async_modules.contains(&module) {
91-
async_modules.insert(parent_module);
92-
}
93-
Ok(())
86+
// child is the previously visited module which must be async
87+
// parent is a new module that depends on it
88+
|child, parent, _state| {
89+
Ok(if let Some((_, edge)) = child {
90+
if edge.chunking_type.is_inherit_async() {
91+
async_modules.insert(parent);
92+
GraphTraversalAction::Continue
93+
} else {
94+
// Wrong edge type to follow
95+
GraphTraversalAction::Exclude
96+
}
97+
} else {
98+
// These are our entry points, just continue
99+
GraphTraversalAction::Continue
100+
})
94101
},
102+
|_, _, _| Ok(()),
95103
)?;
96104

97-
graph.traverse_cycles(
98-
|ref_data| ref_data.chunking_type.is_inherit_async(),
99-
|cycle| {
100-
if cycle.iter().any(|node| async_modules.contains(node)) {
101-
async_modules.extend(cycle.iter().map(|n| **n));
102-
}
103-
Ok(())
104-
},
105-
)?;
105+
// Accumulate the parent modules at the end. Not all parent async modules were in this graph
106+
async_modules.extend(parent_async_modules);
106107

107108
Ok(Vc::cell(async_modules))
108109
}

0 commit comments

Comments
 (0)