Skip to content

Commit e2422f4

Browse files
authored
[turbopack] Add FileSystemPath.has_extension and optimize module graph operations (#81205)
## Optimize file extension checks and module graph traversal ### What? This PR introduces several optimizations: 1. Added a new `has_extension()` method to `FileSystemPath` for more efficient extension checking 2. Improved module graph traversal with a dedicated `has_entry_module()` method 3. Reordered code in `AppProject` to avoid unnecessary clones 4. Simplified CSS resource handling in client reference manifest generation ### Why? These changes improve performance by: - Replacing string comparisons with more efficient suffix matching for file extensions - Providing a direct way to check if a module is an entry point without iterating all entries - Avoiding unnecessary clones operations when possible
1 parent 8ecf21e commit e2422f4

File tree

10 files changed

+101
-98
lines changed

10 files changed

+101
-98
lines changed

crates/next-api/src/app.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -861,14 +861,13 @@ impl AppProject {
861861
client_shared_entries: Vc<EvaluatableAssets>,
862862
has_layout_segments: bool,
863863
) -> Result<Vc<ModuleGraphs>> {
864-
let client_shared_entries = client_shared_entries
865-
.await?
866-
.into_iter()
867-
.map(|m| ResolvedVc::upcast(*m))
868-
.collect();
869-
870-
let should_trace = self.project.next_mode().await?.is_production();
871864
if *self.project.per_page_module_graph().await? {
865+
let should_trace = self.project.next_mode().await?.is_production();
866+
let client_shared_entries = client_shared_entries
867+
.await?
868+
.into_iter()
869+
.map(|m| ResolvedVc::upcast(*m))
870+
.collect();
872871
// Implements layout segment optimization to compute a graph "chain" for each layout
873872
// segment
874873
async move {
@@ -1255,7 +1254,7 @@ impl AppEndpoint {
12551254
client_assets.insert(chunk);
12561255

12571256
let chunk_path = chunk.path().await?;
1258-
if chunk_path.extension_ref() == Some("js") {
1257+
if chunk_path.has_extension(".js") {
12591258
client_shared_chunks.push(chunk);
12601259
}
12611260
}

crates/next-api/src/module_graph.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl NextDynamicGraph {
8282
}
8383

8484
let entries = if !self.is_single_page {
85-
if !graph.entry_modules().any(|m| m == entry) {
85+
if !graph.has_entry_module(entry) {
8686
// the graph doesn't contain the entry, e.g. for the additional module graph
8787
return Ok(Vc::cell(vec![]));
8888
}
@@ -185,7 +185,7 @@ impl ServerActionsGraph {
185185
// The graph contains the whole app, traverse and collect all reachable imports.
186186
let graph = &*self.graph.await?;
187187

188-
if !graph.entry_modules().any(|m| m == entry) {
188+
if !graph.has_entry_module(entry) {
189189
// the graph doesn't contain the entry, e.g. for the additional module graph
190190
return Ok(Vc::cell(Default::default()));
191191
}
@@ -266,7 +266,7 @@ impl ClientReferencesGraph {
266266
}
267267

268268
#[turbo_tasks::function]
269-
pub async fn get_client_references_for_endpoint(
269+
async fn get_client_references_for_endpoint(
270270
&self,
271271
entry: ResolvedVc<Box<dyn Module>>,
272272
) -> Result<Vc<ClientReferenceGraphResult>> {
@@ -276,7 +276,7 @@ impl ClientReferencesGraph {
276276
let graph = &*self.graph.await?;
277277

278278
let entries = if !self.is_single_page {
279-
if !graph.entry_modules().any(|m| m == entry) {
279+
if !graph.has_entry_module(entry) {
280280
// the graph doesn't contain the entry, e.g. for the additional module graph
281281
return Ok(ClientReferenceGraphResult::default().cell());
282282
}
@@ -467,9 +467,7 @@ async fn validate_pages_css_imports(
467467
let module_name_map = module_name_map.await?;
468468

469469
let entries = if !is_single_page {
470-
// TODO: Optimize this code by checking if the node is an entry using `get_module` and then
471-
// checking if the node is an entry in the graph by looking for the reverse edges.
472-
if !graph.entry_modules().any(|m| m == entry) {
470+
if !graph.has_entry_module(entry) {
473471
// the graph doesn't contain the entry, e.g. for the additional module graph
474472
return Ok(());
475473
}

crates/next-api/src/nft_json.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ impl Asset for NftJsonAsset {
218218
}
219219

220220
let referenced_chunk_path = referenced_chunk.path().await?;
221-
if referenced_chunk_path.extension_ref() == Some("map") {
221+
if referenced_chunk_path.has_extension(".map") {
222222
continue;
223223
}
224224

crates/next-core/src/app_page_loader_tree.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,10 @@ impl AppPageLoaderTreeBuilder {
268268
writeln!(self.loader_tree_code, "{s} width: {identifier}.width,")?;
269269
writeln!(self.loader_tree_code, "{s} height: {identifier}.height,")?;
270270
} else {
271-
let ext = path.extension();
272271
// For SVGs, skip sizes and use "any" to let it scale automatically based on viewport,
273272
// For the images doesn't provide the size properly, use "any" as well.
274273
// If the size is presented, use the actual size for the image.
275-
let sizes = if ext == "svg" {
274+
let sizes = if path.has_extension(".svg") {
276275
"any".to_string()
277276
} else {
278277
format!("${{{identifier}.width}}x${{{identifier}.height}}")

crates/next-core/src/next_app/app_client_references_chunks.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ pub struct ClientReferencesChunks {
3232

3333
/// Computes all client references chunks.
3434
///
35-
/// This returns a map from client reference type to the chunks that reference
35+
/// This returns a map from client reference type to the chunks that the reference
3636
/// type needs to load.
3737
#[turbo_tasks::function]
3838
pub async fn get_app_client_references_chunks(

crates/next-core/src/next_client_reference/visit_client_reference.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -218,26 +218,22 @@ impl Visit<FindServerEntriesNode> for FindServerEntries {
218218
}
219219

220220
fn edges(&mut self, node: &FindServerEntriesNode) -> Self::EdgesFuture {
221-
let node = node.clone();
222221
let include_traced = self.include_traced;
222+
let parent_module = match node {
223+
// This should never occur since we always skip visiting these
224+
// nodes' edges.
225+
FindServerEntriesNode::ClientReference => {
226+
unreachable!("ClientReference node should not be visited")
227+
}
228+
FindServerEntriesNode::Internal(module, _) => **module,
229+
FindServerEntriesNode::ServerUtilEntry(module, _) => Vc::upcast(**module),
230+
FindServerEntriesNode::ServerComponentEntry(module, _) => Vc::upcast(**module),
231+
};
223232
async move {
224-
let parent_module = match node {
225-
// This should never occur since we always skip visiting these
226-
// nodes' edges.
227-
FindServerEntriesNode::ClientReference => {
228-
unreachable!("ClientReference node should not be visited")
229-
}
230-
FindServerEntriesNode::Internal(module, _) => module,
231-
FindServerEntriesNode::ServerUtilEntry(module, _) => ResolvedVc::upcast(module),
232-
FindServerEntriesNode::ServerComponentEntry(module, _) => {
233-
ResolvedVc::upcast(module)
234-
}
235-
};
236-
237233
// Pass include_traced to reuse the same cached `primary_chunkable_referenced_modules`
238234
// task result, but the traced references will be filtered out again afterwards.
239235
let referenced_modules =
240-
primary_chunkable_referenced_modules(*parent_module, include_traced).await?;
236+
primary_chunkable_referenced_modules(parent_module, include_traced).await?;
241237

242238
let referenced_modules = referenced_modules
243239
.iter()

crates/next-core/src/next_manifests/client_reference_manifest.rs

Lines changed: 27 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -331,60 +331,52 @@ impl ClientReferenceManifest {
331331
.value_to_string()
332332
.owned()
333333
.await?;
334-
let mut entry_css_files_with_chunk = Vec::new();
335334
let entry_js_files = entry_manifest
336335
.entry_js_files
337336
.entry(server_component_name.clone())
338337
.or_default();
338+
let entry_css_files = entry_manifest
339+
.entry_css_files
340+
.entry(server_component_name)
341+
.or_default();
339342

340343
let client_chunks = &client_chunks.await?;
341344
let client_chunks_with_path =
342345
cached_chunk_paths(&mut client_chunk_path_cache, client_chunks.iter().copied())
343346
.await?;
347+
// Inlining breaks HMR so it is always disabled in dev.
348+
let inlined_css = next_config.await?.experimental.inline_css.unwrap_or(false)
349+
&& mode.is_production();
344350

345351
for (chunk, chunk_path) in client_chunks_with_path {
346352
if let Some(path) = client_relative_path.get_path_to(&chunk_path) {
347353
// The entry CSS files and entry JS files don't have prefix and suffix
348-
// applied because it is added by Nex.js during rendering.
354+
// applied because it is added by Next.js during rendering.
349355
let path = path.into();
350-
if chunk_path.extension_ref() == Some("css") {
351-
entry_css_files_with_chunk.push((path, chunk));
356+
if chunk_path.has_extension(".css") {
357+
let content = if inlined_css {
358+
Some(
359+
if let Some(content_file) =
360+
chunk.content().file_content().await?.as_content()
361+
{
362+
content_file.content().to_str()?.into()
363+
} else {
364+
RcStr::default()
365+
},
366+
)
367+
} else {
368+
None
369+
};
370+
entry_css_files.insert(CssResource {
371+
path,
372+
inlined: inlined_css,
373+
content,
374+
});
352375
} else {
353376
entry_js_files.insert(path);
354377
}
355378
}
356379
}
357-
358-
let inlined = next_config.await?.experimental.inline_css.unwrap_or(false)
359-
&& mode.is_production();
360-
let entry_css_files_vec = entry_css_files_with_chunk
361-
.into_iter()
362-
.map(async |(path, chunk)| {
363-
let content = if inlined {
364-
if let Some(content_file) =
365-
chunk.content().file_content().await?.as_content()
366-
{
367-
Some(content_file.content().to_str()?.into())
368-
} else {
369-
Some("".into())
370-
}
371-
} else {
372-
None
373-
};
374-
Ok(CssResource {
375-
path,
376-
inlined,
377-
content,
378-
})
379-
})
380-
.try_join()
381-
.await?;
382-
383-
let entry_css_files = entry_manifest
384-
.entry_css_files
385-
.entry(server_component_name)
386-
.or_default();
387-
entry_css_files.extend(entry_css_files_vec);
388380
}
389381

390382
let client_reference_manifest_json = serde_json::to_string(&entry_manifest).unwrap();

crates/next-core/src/next_server/resolve.rs

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -169,35 +169,32 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
169169
) -> Result<FileType> {
170170
// node.js only supports these file extensions
171171
// mjs is an esm module and we can't bundle that yet
172-
let ext = raw_fs_path.extension_ref();
173-
if matches!(ext, Some("cjs" | "node" | "json")) {
174-
return Ok(FileType::CommonJs);
175-
}
176-
if matches!(ext, Some("mjs")) {
177-
return Ok(FileType::EcmaScriptModule);
178-
}
179-
if matches!(ext, Some("js")) {
180-
// for .js extension in cjs context, we need to check the actual module type via
181-
// package.json
182-
let FindContextFileResult::Found(package_json, _) =
183-
&*find_context_file(fs_path.parent(), package_json()).await?
184-
else {
185-
// can't find package.json
186-
return Ok(FileType::CommonJs);
187-
};
188-
let FileJsonContent::Content(package) = &*package_json.read_json().await? else {
189-
// can't parse package.json
190-
return Ok(FileType::InvalidPackageJson);
191-
};
192-
193-
if let Some("module") = package["type"].as_str() {
194-
return Ok(FileType::EcmaScriptModule);
172+
Ok(match raw_fs_path.extension_ref() {
173+
Some("cjs" | "node" | "json") => FileType::CommonJs,
174+
Some("mjs") => FileType::EcmaScriptModule,
175+
Some("js") => {
176+
// for .js extension in cjs context, we need to check the actual module type via
177+
// package.json
178+
let FindContextFileResult::Found(package_json, _) =
179+
&*find_context_file(fs_path.parent(), package_json()).await?
180+
else {
181+
// can't find package.json
182+
return Ok(FileType::CommonJs);
183+
};
184+
let FileJsonContent::Content(package) = &*package_json.read_json().await?
185+
else {
186+
// can't parse package.json
187+
return Ok(FileType::InvalidPackageJson);
188+
};
189+
190+
if let Some("module") = package["type"].as_str() {
191+
FileType::EcmaScriptModule
192+
} else {
193+
FileType::CommonJs
194+
}
195195
}
196-
197-
return Ok(FileType::CommonJs);
198-
}
199-
200-
Ok(FileType::UnsupportedExtension)
196+
_ => FileType::UnsupportedExtension,
197+
})
201198
}
202199

203200
let unable_to_externalize = |reason: Vec<StyledString>| {

turbopack/crates/turbo-tasks-fs/src/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,16 @@ impl FileSystemPath {
11101110
file_name
11111111
}
11121112

1113+
/// Returns true if this path has the given extension
1114+
///
1115+
/// slightly faster than `self.extension_ref() == Some(extension)` as we can simply match a
1116+
/// suffix
1117+
pub fn has_extension(&self, extension: &str) -> bool {
1118+
debug_assert!(!extension.contains('/') && extension.starts_with('.'));
1119+
self.path.ends_with(extension)
1120+
}
1121+
1122+
/// Returns the extension (without a leading `.`)
11131123
pub fn extension_ref(&self) -> Option<&str> {
11141124
let (_, extension) = self.split_extension();
11151125
extension

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,19 @@ impl SingleModuleGraph {
399399
})
400400
}
401401

402-
/// Iterate over all nodes in the graph
402+
/// Returns true if the given module is in this graph and is an entry module
403+
pub fn has_entry_module(&self, module: ResolvedVc<Box<dyn Module>>) -> bool {
404+
if let Some(index) = self.modules.get(&module) {
405+
self.graph
406+
.edges_directed(*index, petgraph::Direction::Incoming)
407+
.next()
408+
.is_none()
409+
} else {
410+
false
411+
}
412+
}
413+
414+
/// Iterate over graph entry points
403415
pub fn entry_modules(&self) -> impl Iterator<Item = ResolvedVc<Box<dyn Module>>> + '_ {
404416
self.entries.iter().flat_map(|e| e.entries())
405417
}

0 commit comments

Comments
 (0)