Skip to content

Commit

Permalink
chore(turbopack): Remove or improve a few uses of `.to_resolved().awa…
Browse files Browse the repository at this point in the history
…it` inside a loop (#74112)

- We should generally prefer `try_join` over `to_resolved().await` in a loop as it can provide more parallelism.
- Some of these should just resolve the `Vc` outside the loop.
  • Loading branch information
bgw authored Dec 20, 2024
1 parent afddbe7 commit bc69d97
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,14 @@ impl OutputAsset for EcmascriptDevEvaluateChunk {
.await?,
);

for chunk in &*self.other_chunks.await? {
ident.add_modifier(chunk.ident().to_string().to_resolved().await?);
}
ident.modifiers.extend(
self.other_chunks
.await?
.iter()
.map(|chunk| chunk.ident().to_string().to_resolved())
.try_join()
.await?,
);

let ident = AssetIdent::new(Value::new(ident));
Ok(AssetIdent::from_path(
Expand Down
12 changes: 5 additions & 7 deletions turbopack/crates/turbopack-core/src/introspect/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub async fn children_from_module_references(
}
}

let key = key.to_resolved().await?;
for &module in reference
.resolve_reference()
.resolve()
Expand All @@ -100,18 +101,15 @@ pub async fn children_from_module_references(
.await?
.iter()
{
children.insert((key.to_resolved().await?, IntrospectableModule::new(*module)));
children.insert((key, IntrospectableModule::new(*module)));
}
for &output_asset in reference
.resolve_reference()
.primary_output_assets()
.await?
.iter()
{
children.insert((
key.to_resolved().await?,
IntrospectableOutputAsset::new(*output_asset),
));
children.insert((key, IntrospectableOutputAsset::new(*output_asset)));
}
}

Check notice on line 114 in turbopack/crates/turbopack-core/src/introspect/utils.rs

View workflow job for this annotation

GitHub Actions / ast-grep lint

to-resolved-in-loop

Calling `key.to_resolved().await?` in a loop could be a doing a lot of work sequentially. Consider producing an iterator of futures and using `try_join`.
Ok(Vc::cell(children))
Expand All @@ -121,12 +119,12 @@ pub async fn children_from_module_references(
pub async fn children_from_output_assets(
references: Vc<OutputAssets>,
) -> Result<Vc<IntrospectableChildren>> {
let key = reference_ty();
let key = reference_ty().to_resolved().await?;
let mut children = FxIndexSet::default();
let references = references.await?;
for &reference in &*references {
children.insert((
key.to_resolved().await?,
key,
IntrospectableOutputAsset::new(*ResolvedVc::upcast(reference)),
));
}
Expand Down
18 changes: 11 additions & 7 deletions turbopack/crates/turbopack-css/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ fn chunk_item_key() -> Vc<RcStr> {
impl OutputAsset for CssChunk {
#[turbo_tasks::function]
async fn ident(&self) -> Result<Vc<AssetIdent>> {
let mut assets = Vec::new();

let CssChunkContent { chunk_items, .. } = &*self.content.await?;
let mut common_path = if let Some(chunk_item) = chunk_items.first() {
let path = chunk_item.asset_ident().path().to_resolved().await?;
Expand All @@ -266,7 +264,7 @@ impl OutputAsset for CssChunk {

// The included chunk items and the availability info describe the chunk
// uniquely
let chunk_item_key = chunk_item_key();
let chunk_item_key = chunk_item_key().to_resolved().await?;
for &chunk_item in chunk_items.iter() {
if let Some((common_path_vc, common_path_ref)) = common_path.as_mut() {
let path = chunk_item.asset_ident().path().await?;
Expand All @@ -280,11 +278,17 @@ impl OutputAsset for CssChunk {
*common_path_ref = (*common_path_vc).await?;
}
}
assets.push((
chunk_item_key.to_resolved().await?,
chunk_item.content_ident().to_resolved().await?,
));
}
let assets = chunk_items
.iter()
.map(|chunk_item| async move {
Ok((
chunk_item_key,
chunk_item.content_ident().to_resolved().await?,
))
})
.try_join()
.await?;

let ident = AssetIdent {
path: if let Some((common_path, _)) = common_path {
Expand Down
19 changes: 11 additions & 8 deletions turbopack/crates/turbopack-ecmascript/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ fn availability_root_key() -> Vc<RcStr> {
impl Chunk for EcmascriptChunk {
#[turbo_tasks::function]
async fn ident(&self) -> Result<Vc<AssetIdent>> {
let mut assets = Vec::new();

let EcmascriptChunkContent { chunk_items, .. } = &*self.content.await?;
let mut common_path = if let Some((chunk_item, _)) = chunk_items.first() {
let path = chunk_item.asset_ident().path().to_resolved().await?;
Expand All @@ -86,7 +84,6 @@ impl Chunk for EcmascriptChunk {
};

// The included chunk items describe the chunk uniquely
let chunk_item_key = chunk_item_key();
for &(chunk_item, _) in chunk_items.iter() {
if let Some((common_path_vc, common_path_ref)) = common_path.as_mut() {
let path = chunk_item.asset_ident().path().await?;
Expand All @@ -100,13 +97,19 @@ impl Chunk for EcmascriptChunk {
*common_path_ref = (*common_path_vc).await?;
}
}
assets.push((
chunk_item_key.to_resolved().await?,
chunk_item.content_ident().to_resolved().await?,
));
}

// The previous resolve loop is no longer needed since we're already using ResolvedVc
let chunk_item_key = chunk_item_key().to_resolved().await?;
let assets = chunk_items
.iter()
.map(|&(chunk_item, _)| async move {
Ok((
chunk_item_key,
chunk_item.content_ident().to_resolved().await?,
))
})
.try_join()
.await?;

let ident = AssetIdent {
path: if let Some((common_path, _)) = common_path {
Expand Down

0 comments on commit bc69d97

Please sign in to comment.