Skip to content

Turbopack: perf: Fix unused argument filtering optimization in turbo-tasks for self arguments #86231

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 20, 2025
+119 −71
Merged

Turbopack: perf: Fix unused argument filtering optimization in turbo-tasks for self arguments #86231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ impl Adder {

#[turbo_tasks::function]
async fn add_method(&self, arg1: ResolvedVc<u16>, arg2: ResolvedVc<u32>) -> Result<Vc<u64>> {
let _ = self; // Make sure unused argument filtering doesn't remove the arg
Ok(Vc::cell(u64::from(*arg1.await?) + u64::from(*arg2.await?)))
}
}
Expand Down
3 changes: 3 additions & 0 deletions turbopack/crates/turbo-tasks-macros/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ quote = { workspace = true }
regex = { workspace = true }
rustc-hash = { workspace = true }
syn = { workspace = true, features = ["full", "extra-traits", "visit", "visit-mut"] }

[dev-dependencies]
rstest = { workspace = true }
16 changes: 9 additions & 7 deletions turbopack/crates/turbo-tasks-macros/src/func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub struct TurboFn<'a> {

output: Type,
this: Option<Input>,
is_self_used: bool,
exposed_inputs: Vec<Input>,
/// Should we return `OperationVc` and require that all arguments are `NonLocalValue`s?
operation: bool,
Expand All @@ -43,6 +44,7 @@ impl TurboFn<'_> {
orig_signature: &Signature,
definition_context: DefinitionContext,
args: FunctionArguments,
is_self_used: bool,
) -> Option<TurboFn<'_>> {
if !orig_signature.generics.params.is_empty() {
orig_signature
Expand Down Expand Up @@ -202,6 +204,7 @@ impl TurboFn<'_> {
ident: orig_ident,
output,
this,
is_self_used,
exposed_inputs,
operation: args.operation.is_some(),
inline_ident,
Expand Down Expand Up @@ -271,7 +274,6 @@ impl TurboFn<'_> {
pub fn inline_signature_and_block<'a>(
&self,
orig_block: &'a Block,
is_self_used: bool,
) -> (Signature, Cow<'a, Block>) {
let mut shadow_self = None;
let (inputs, transform_stmts): (Punctuated<_, _>, Vec<Option<_>>) = self
Expand All @@ -280,7 +282,7 @@ impl TurboFn<'_> {
.iter()
.filter(|arg| {
let FnArg::Typed(pat_type) = arg else {
return is_self_used;
return self.is_self_used;
};
let Pat::Ident(pat_id) = &*pat_type.pat else {
return true;
Expand Down Expand Up @@ -543,8 +545,7 @@ impl TurboFn<'_> {
}
}

/// The block of the exposed function for a dynamic dispatch call to the
/// given trait.
/// The block of the exposed function for a dynamic dispatch call to the given trait.
pub fn dynamic_block(&self, trait_type_ident: &Ident) -> Block {
let Some(converted_this) = self.converted_this() else {
return parse_quote! {
Expand Down Expand Up @@ -579,13 +580,14 @@ impl TurboFn<'_> {
}
}

/// The block of the exposed function for a static dispatch call to the
/// given native function.
/// The block of the exposed function for a static dispatch call to the given native function.
pub fn static_block(&self, native_function_ident: &Ident) -> Block {
let output = &self.output;
let inputs = self.inline_input_idents();
let assertions = self.get_assertions();
let mut block = if let Some(converted_this) = self.converted_this() {
let mut block = if self.is_self_used
&& let Some(converted_this) = self.converted_this()
{
let persistence = self.persistence_with_this();
parse_quote! {
{
Expand Down
5 changes: 2 additions & 3 deletions turbopack/crates/turbo-tasks-macros/src/function_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
.unwrap_or_default();
let is_self_used = args.operation.is_some() || is_self_used(&block);

let Some(turbo_fn) = TurboFn::new(&sig, DefinitionContext::NakedFn, args) else {
let Some(turbo_fn) = TurboFn::new(&sig, DefinitionContext::NakedFn, args, is_self_used) else {
return quote! {
// An error occurred while parsing the function signature.
}
Expand All @@ -53,8 +53,7 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
let ident = &sig.ident;

let inline_function_ident = turbo_fn.inline_ident();
let (inline_signature, inline_block) =
turbo_fn.inline_signature_and_block(&block, is_self_used);
let (inline_signature, inline_block) = turbo_fn.inline_signature_and_block(&block);
let inline_attrs = filter_inline_attributes(&attrs[..]);
let function_path_string = ident.to_string();

Expand Down
35 changes: 35 additions & 0 deletions turbopack/crates/turbo-tasks-macros/src/self_filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,38 @@ fn contains_self_token(tok: &TokenTree) -> bool {
TokenTree::Punct(..) | TokenTree::Literal(..) => false,
}
}

#[cfg(test)]
mod tests {
use rstest::rstest;

use super::*;

#[rstest]
#[case::no_self_usage(r#"{ let x = 42; println!("hello"); }"#, false)]
#[case::simple_self_usage(r#"{ self.foo(); }"#, true)]
#[case::self_field_access(r#"{ let x = self.field; }"#, true)]
#[case::self_in_nested_block(r#"{ let x = 1; { self.method(); } }"#, true)]
#[case::self_in_impl_block_not_detected(
r#"{ impl Foo { fn bar(&self) { self.baz(); } } }"#,
false
)]
#[case::self_before_impl_block(
r#"{ self.foo(); impl Bar { fn baz(&self) { self.qux(); } } }"#,
true
)]
#[case::self_in_closure(r#"{ let f = || { self.method(); }; }"#, true)]
#[case::self_in_if_condition(r#"{ if self.check() { println!("true"); } }"#, true)]
#[case::self_in_match_arm(r#"{ match x { Some(_) => self.handle(), None => {}, } }"#, true)]
#[case::self_in_macro(r#"{ println!("{:?}", self); }"#, true)]
#[case::self_in_complex_macro(r#"{ format!("value: {}", self.field); }"#, true)]
#[case::no_self_with_similar_idents(r#"{ let myself = 42; let selfish = true; }"#, false)]
#[case::empty_block(r#"{}"#, false)]
#[case::self_in_return_statement(r#"{ return self.value; }"#, true)]
#[case::self_as_function_argument(r#"{ some_function(self); }"#, true)]
fn test_is_self_used(#[case] code: &str, #[case] expected: bool) {
let block: syn::Block = syn::parse_str(code).unwrap();
let result = is_self_used(&block);
assert_eq!(result, expected);
}
}
123 changes: 65 additions & 58 deletions turbopack/crates/turbo-tasks-macros/src/value_impl_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,66 +73,71 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
let mut errors = Vec::new();

for item in items.iter() {
if let ImplItem::Fn(ImplItemFn {
let ImplItem::Fn(ImplItemFn {
attrs,
vis,
defaultness: _,
sig,
block,
}) = item
{
let ident = &sig.ident;
let (func_args, attrs) = split_function_attributes(attrs);
let func_args = match func_args {
Ok(None) => {
item.span()
.unwrap()
.error("#[turbo_tasks::function] attribute missing")
.emit();
FunctionArguments::default()
}
Ok(Some(func_args)) => func_args,
Err(error) => {
errors.push(error.to_compile_error());
FunctionArguments::default()
}
};
let is_self_used = func_args.operation.is_some() || is_self_used(block);

let Some(turbo_fn) =
TurboFn::new(sig, DefinitionContext::ValueInherentImpl, func_args)
else {
return quote! {
// An error occurred while parsing the function signature.
};
};
else {
continue;
};

let ident = &sig.ident;
let (func_args, attrs) = split_function_attributes(attrs);
let func_args = match func_args {
Ok(None) => {
item.span()
.unwrap()
.error("#[turbo_tasks::function] attribute missing")
.emit();
FunctionArguments::default()
}
Ok(Some(func_args)) => func_args,
Err(error) => {
errors.push(error.to_compile_error());
FunctionArguments::default()
}
};
let is_self_used = func_args.operation.is_some() || is_self_used(block);

let inline_function_ident = turbo_fn.inline_ident();
let (inline_signature, inline_block) =
turbo_fn.inline_signature_and_block(block, is_self_used);
let inline_attrs = filter_inline_attributes(attrs.iter().copied());
let function_path_string = format!("{ty}::{ident}", ty = ty.to_token_stream());
let native_fn = NativeFn {
function_global_name: global_name(&function_path_string),
function_path_string,
function_path: parse_quote! { <#ty>::#inline_function_ident },
is_method: turbo_fn.is_method(),
is_self_used,
filter_trait_call_args: None, // not a trait method
let Some(turbo_fn) = TurboFn::new(
sig,
DefinitionContext::ValueInherentImpl,
func_args,
is_self_used,
) else {
return quote! {
// An error occurred while parsing the function signature.
};

let native_function_ident = get_inherent_impl_function_ident(ty_ident, ident);
let native_function_ty = native_fn.ty();
let native_function_def = native_fn.definition();

let turbo_signature = turbo_fn.signature();
let turbo_block = turbo_fn.static_block(&native_function_ident);
exposed_impl_items.push(quote! {
#(#attrs)*
#vis #turbo_signature #turbo_block
});

all_definitions.push(quote! {
};

let inline_function_ident = turbo_fn.inline_ident();
let (inline_signature, inline_block) = turbo_fn.inline_signature_and_block(block);
let inline_attrs = filter_inline_attributes(attrs.iter().copied());
let function_path_string = format!("{ty}::{ident}", ty = ty.to_token_stream());
let native_fn = NativeFn {
function_global_name: global_name(&function_path_string),
function_path_string,
function_path: parse_quote! { <#ty>::#inline_function_ident },
is_method: turbo_fn.is_method(),
is_self_used,
filter_trait_call_args: None, // not a trait method
};

let native_function_ident = get_inherent_impl_function_ident(ty_ident, ident);
let native_function_ty = native_fn.ty();
let native_function_def = native_fn.definition();

let turbo_signature = turbo_fn.signature();
let turbo_block = turbo_fn.static_block(&native_function_ident);
exposed_impl_items.push(quote! {
#(#attrs)*
#vis #turbo_signature #turbo_block
});

all_definitions.push(quote! {
#[doc(hidden)]
impl #ty {
// By declaring the native function's body within an `impl` block, we ensure
Expand All @@ -153,7 +158,6 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
turbo_tasks::macro_helpers::CollectableFunction(&#native_function_ident)
}
})
}
}

quote! {
Expand Down Expand Up @@ -207,11 +211,15 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
continue;
}
};
// operations are not currently compatible with methods
let is_self_used = func_args.operation.is_some() || is_self_used(block);

let Some(turbo_fn) =
TurboFn::new(sig, DefinitionContext::ValueTraitImpl, func_args)
else {
let Some(turbo_fn) = TurboFn::new(
sig,
DefinitionContext::ValueTraitImpl,
func_args,
is_self_used,
) else {
return quote! {
// An error occurred while parsing the function signature.
};
Expand All @@ -222,8 +230,7 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
&format!("{ty_ident}_{trait_ident}_{ident}_inline"),
ident.span(),
);
let (inline_signature, inline_block) =
turbo_fn.inline_signature_and_block(block, is_self_used);
let (inline_signature, inline_block) = turbo_fn.inline_signature_and_block(block);
let inline_attrs = filter_inline_attributes(attrs.iter().copied());
let native_fn = NativeFn {
// This global name breaks the pattern. It isn't clear if it is intentional
Expand Down
6 changes: 3 additions & 3 deletions turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,12 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
.emit();
}

let is_self_used = default.as_ref().map(is_self_used).unwrap_or(false);
let Some(turbo_fn) = TurboFn::new(
sig,
DefinitionContext::ValueTrait,
FunctionArguments::default(),
is_self_used,
) else {
return quote! {
// An error occurred while parsing the function signature.
Expand All @@ -194,12 +196,10 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
});

let default = if let Some(default) = default {
let is_self_used = is_self_used(default);
let inline_function_ident = turbo_fn.inline_ident();
let inline_extension_trait_ident =
Ident::new(&format!("{trait_ident}_{ident}_inline"), ident.span());
let (inline_signature, inline_block) =
turbo_fn.inline_signature_and_block(default, is_self_used);
let (inline_signature, inline_block) = turbo_fn.inline_signature_and_block(default);
let inline_attrs = filter_inline_attributes(attrs.iter().copied());

let function_path_string = format!("{trait_ident}::{ident}");
Expand Down
Loading