Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce number of task in async_module #5959

Merged
merged 1 commit into from
Sep 15, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 10 additions & 17 deletions crates/turbopack-ecmascript/src/references/async_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use swc_core::{
ecma::ast::{ArrayLit, ArrayPat, Expr, Ident, Pat, Program},
quote,
};
use turbo_tasks::{primitives::Bools, trace::TraceRawVcs, TryFlatJoinIterExt, Value, Vc};
use turbo_tasks::{trace::TraceRawVcs, TryFlatJoinIterExt, Value, Vc};
use turbopack_core::chunk::availability_info::AvailabilityInfo;

use super::esm::base::ReferencedAsset;
Expand Down Expand Up @@ -102,17 +102,6 @@ struct AsyncModuleScc {
#[turbo_tasks::value(transparent)]
struct OptionAsyncModuleScc(Option<Vc<AsyncModuleScc>>);

#[turbo_tasks::function]
async fn is_placeable_self_async(
placeable: Vc<Box<dyn EcmascriptChunkPlaceable>>,
) -> Result<Vc<bool>> {
let Some(async_module) = &*placeable.get_async_module().await? else {
return Ok(Vc::cell(false));
};

Ok(async_module.is_self_async())
}

#[turbo_tasks::value_impl]
impl AsyncModuleScc {
#[turbo_tasks::function]
Expand All @@ -124,19 +113,23 @@ impl AsyncModuleScc {
async fn is_async(self: Vc<Self>) -> Result<Vc<bool>> {
let this = self.await?;

let mut bools = Vec::new();

for placeable in &*this.scc.await? {
bools.push(is_placeable_self_async(*placeable));
if let Some(async_module) = &*placeable.get_async_module().await? {
if *async_module.is_self_async().await? {
return Ok(Vc::cell(true));
}
}
}

for scc in &*this.scope.get_scc_children(this.scc).await? {
// Because we generated SCCs there can be no loops in the children, so calling
// recursively is fine.
bools.push(AsyncModuleScc::new(*scc, this.scope).is_async());
if *AsyncModuleScc::new(*scc, this.scope).is_async().await? {
return Ok(Vc::cell(true));
}
}

Ok(Vc::<Bools>::cell(bools).any())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this faster because we've implemented early exiting, or because we no longer need to build up this Bools vc just to await it? Does this have implications for the just-approved https://github.com/vercel/next.js/blob/35fd8bcb0388420618a6ffb08ad5e93f716016c1/packages/next-swc/crates/next-api/src/project.rs#L706

Copy link
Member Author

@sokra sokra Sep 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

early exist reduces the number of tasks. The Bools::any() also saves 1 task, but it's only a single one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completions is technically similar, but no early exit is possible here. We could avoid 1 task by not using Completions::completed(), but it's only 1 task and it has effects on the invalidation: Without Completions::completed() it would need to rerun the current function when any completion fires. With Completions::completed() it only reruns Completions::completed() and doesn't need to rerun the current function.

There is a difference in how often these methods are called is_async is called many many times (for every module), while any_output_changed is only called once.

Ok(Vc::cell(false))
}
}

Expand Down