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

perf(turbo-tasks): Filter out and do not cache unused arguments #75261

Draft
wants to merge 2 commits into
base: canary
Choose a base branch
from
Draft
Show file tree
Hide file tree
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
95 changes: 66 additions & 29 deletions turbopack/crates/turbo-tasks-macros/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

output: Type,
this: Option<Input>,
inputs: Vec<Input>,
exposed_inputs: Vec<Input>,
/// Should we return `OperationVc` and require that all arguments are `NonLocalValue`s?
operation: bool,
/// Should this function use `TaskPersistence::LocalCells`?
Expand Down Expand Up @@ -75,7 +75,7 @@

let mut raw_inputs = orig_signature.inputs.iter();
let mut this = None;
let mut inputs = Vec::with_capacity(raw_inputs.len());
let mut exposed_inputs = Vec::with_capacity(raw_inputs.len());

if let Some(possibly_receiver) = raw_inputs.next() {
match possibly_receiver {
Expand Down Expand Up @@ -218,7 +218,7 @@
}
let ident = ident.ident.clone();

inputs.push(Input {
exposed_inputs.push(Input {
ident,
ty: (*typed.ty).clone(),
});
Expand All @@ -227,7 +227,7 @@
// We can't support destructuring patterns (or other kinds of patterns).
let ident = Ident::new("arg1", typed.pat.span());

inputs.push(Input {
exposed_inputs.push(Input {
ident,
ty: (*typed.ty).clone(),
});
Expand All @@ -249,7 +249,7 @@
Ident::new(&format!("arg{}", i + 2), typed.pat.span())
};

inputs.push(Input {
exposed_inputs.push(Input {
ident,
ty: (*typed.ty).clone(),
});
Expand All @@ -272,7 +272,7 @@
ident: orig_ident,
output,
this,
inputs,
exposed_inputs,
operation: args.operation.is_some(),
local: args.local.is_some(),
inline_ident,
Expand All @@ -286,7 +286,7 @@
.this
.as_ref()
.into_iter()
.chain(self.inputs.iter())
.chain(self.exposed_inputs.iter())
.map(|input| {
FnArg::Typed(PatType {
attrs: Vec::new(),
Expand Down Expand Up @@ -348,6 +348,15 @@
.orig_signature
.inputs
.iter()
.filter(|arg| {
let FnArg::Typed(pat_type) = arg else {
return true;
};
let Pat::Ident(pat_id) = &*pat_type.pat else {
return true;
};
inline_inputs_identifier_filter(&pat_id.ident)
})
.enumerate()
.map(|(idx, arg)| match arg {
FnArg::Receiver(_) => (arg.clone(), None),
Expand Down Expand Up @@ -470,12 +479,18 @@
&self.inline_ident
}

fn input_idents(&self) -> impl Iterator<Item = &Ident> {
self.inputs.iter().map(|Input { ident, .. }| ident)
fn inline_input_idents(&self) -> impl Iterator<Item = &Ident> {
self.exposed_inputs
.iter()
.map(|Input { ident, .. }| ident)
.filter(|id| inline_inputs_identifier_filter(id))
}

pub fn input_types(&self) -> Vec<&Type> {
self.inputs.iter().map(|Input { ty, .. }| ty).collect()
self.exposed_inputs
.iter()
.map(|Input { ty, .. }| ty)
.collect()
}

pub fn persistence(&self) -> impl ToTokens {
Expand Down Expand Up @@ -557,7 +572,7 @@
let ident = &self.ident;
let output = &self.output;
let assertions = self.get_assertions();
let inputs = self.input_idents();
let inputs = self.inline_input_idents();
let persistence = self.persistence_with_this();
parse_quote! {
{
Expand All @@ -582,7 +597,7 @@
/// given native function.
pub fn static_block(&self, native_function_id_ident: &Ident) -> Block {
let output = &self.output;
let inputs = self.input_idents();
let inputs = self.inline_input_idents();
let assertions = self.get_assertions();
let mut block = if let Some(converted_this) = self.converted_this() {
let persistence = self.persistence_with_this();
Expand Down Expand Up @@ -1036,17 +1051,18 @@
pub function_path_string: String,
pub function_path: ExprPath,
pub is_method: bool,
pub filter_trait_call_args: Option<TokenStream>,
pub local: bool,
pub local_cells: bool,
}

impl NativeFn {
pub fn ty(&self) -> Type {
parse_quote! { turbo_tasks::NativeFunction }
parse_quote! { turbo_tasks::macro_helpers::NativeFunction }
}

pub fn definition(&self) -> TokenStream {

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-native / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-wasm (nodejs)

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-wasm (web)

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test unit (18) / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test devlow package / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test unit (20) / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test next-swc wasm / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test cargo benches / Test

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / rust check / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / rustdoc check / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test cargo unit / build

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / stable - x86_64-unknown-linux-gnu - node@16

mismatched types

Check failure on line 1064 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-native-windows / build

mismatched types
let Self {

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-native / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-wasm (nodejs)

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-wasm (web)

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test unit (18) / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test devlow package / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test unit (20) / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test next-swc wasm / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test cargo benches / Test

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / rust check / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / rustdoc check / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / test cargo unit / build

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / stable - x86_64-unknown-linux-gnu - node@16

pattern does not mention field `filter_trait_call_args`

Check failure on line 1065 in turbopack/crates/turbo-tasks-macros/src/func.rs

View workflow job for this annotation

GitHub Actions / build-native-windows / build

pattern does not mention field `filter_trait_call_args`
function_path_string,
function_path,
is_method,
Expand All @@ -1055,24 +1071,40 @@
} = self;

let constructor = if *is_method {
quote! { new_method }
let arg_meta = if let Some(filter) = self.filter_trait_call_args {
quote! { turbo_tasks::macro_helpers::ArgMeta::with_filter_trait_call_args(#filter) }
} else {
quote! { turbo_tasks::macro_helpers::ArgMeta::new() }
};
quote! {
{
#[allow(deprecated)]
turbo_tasks::macro_helpers::NativeFunction::new_method(
#function_path_string.to_owned(),
turbo_tasks::macro_helpers::FunctionMeta {
local: #local,
local_cells: #local_cells,
},
turbo_tasks::macro_helpers::ArgMeta::new(),
#function_path,
)
}
}
} else {
quote! { new_function }
};

quote! {
{
#[allow(deprecated)]
turbo_tasks::NativeFunction::#constructor(
#function_path_string.to_owned(),
turbo_tasks::FunctionMeta {
local: #local,
local_cells: #local_cells,
},
#function_path,
)
quote! {
{
#[allow(deprecated)]
turbo_tasks::macro_helpers::NativeFunction::new_function(
#function_path_string.to_owned(),
turbo_tasks::macro_helpers::FunctionMeta {
local: #local,
local_cells: #local_cells,
},
#function_path,
)
}
}
}
};
}

pub fn id_ty(&self) -> Type {
Expand All @@ -1095,3 +1127,8 @@
.filter(|attr| attr.path.get_ident().is_none_or(|id| id != "doc"))
.collect()
}

pub fn inline_inputs_identifier_filter(arg_ident: &Ident) -> bool {
// filter out underscore-prefixed (unused) arguments, we don't need to cache these
!arg_ident.to_string().starts_with('_')
}
1 change: 0 additions & 1 deletion turbopack/crates/turbo-tasks/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ pub use manager::{
CurrentCellRef, ReadConsistency, TaskPersistence, TurboTasks, TurboTasksApi,
TurboTasksBackendApi, TurboTasksBackendApiExt, TurboTasksCallApi, Unused, UpdateInfo,
};
pub use native_function::{FunctionMeta, NativeFunction};
pub use output::OutputContent;
pub use raw_vc::{CellId, RawVc, ReadRawVcFuture, ResolveTypeError};
pub use read_ref::ReadRef;
Expand Down
7 changes: 5 additions & 2 deletions turbopack/crates/turbo-tasks/src/macro_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ pub use super::{
manager::{find_cell_by_type, notify_scheduled_tasks, spawn_detached_for_testing},
};
use crate::{
debug::ValueDebugFormatString, shrink_to_fit::ShrinkToFit, task::TaskOutput, NonLocalValue,
RawVc, TaskInput, TaskPersistence, Vc,
debug::ValueDebugFormatString,
native_function::{ArgMeta, FunctionMeta, NativeFunction},
shrink_to_fit::ShrinkToFit,
task::TaskOutput,
NonLocalValue, RawVc, TaskInput, TaskPersistence, Vc,
};

#[inline(never)]
Expand Down
5 changes: 3 additions & 2 deletions turbopack/crates/turbo-tasks/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use crate::{
},
id_factory::{IdFactory, IdFactoryWithReuse},
magic_any::MagicAny,
native_function::FunctionMeta,
raw_vc::{CellId, RawVc},
registry::{self, get_function},
serialization_invalidation::SerializationInvalidator,
Expand All @@ -48,8 +49,8 @@ use crate::{
trait_helpers::get_trait_method,
util::StaticOrArc,
vc::ReadVcFuture,
Completion, FunctionMeta, InvalidationReason, InvalidationReasonSet, ResolvedVc,
SharedReference, TaskId, TaskIdSet, ValueTypeId, Vc, VcRead, VcValueTrait, VcValueType,
Completion, InvalidationReason, InvalidationReasonSet, ResolvedVc, SharedReference, TaskId,
TaskIdSet, ValueTypeId, Vc, VcRead, VcValueTrait, VcValueType,
};

pub trait TurboTasksCallApi: Sync + Send {
Expand Down
17 changes: 16 additions & 1 deletion turbopack/crates/turbo-tasks/src/native_function.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fmt::Debug, hash::Hash, pin::Pin};
use std::{env::args, fmt::Debug, hash::Hash, pin::Pin};

use anyhow::{Context, Result};
use futures::Future;
Expand All @@ -23,15 +23,28 @@ type ResolveFunctor =

type IsResolvedFunctor = fn(&dyn MagicAny) -> bool;

type FilterArgsFunctor = for<'a> fn(Box<dyn MagicAny>) -> Box<dyn MagicAny>;

pub struct ArgMeta {
serializer: MagicAnySerializeSeed,
deserializer: MagicAnyDeserializeSeed,
is_resolved: IsResolvedFunctor,
resolve: ResolveFunctor,
filter_trait_call_args: FilterArgsFunctor,
}

impl ArgMeta {
pub fn new<T>() -> Self
where
T: TaskInput + Serialize + for<'de> Deserialize<'de> + 'static,
{
fn noop_filter_args(args: Box<dyn MagicAny>) -> Box<dyn MagicAny> {
args
}
Self::with_filter_trait_call_args::<T>(noop_filter_args)
}

pub fn with_filter_trait_call_args<T>(filter_trait_call_args: FilterArgsFunctor) -> Self
where
T: TaskInput + Serialize + for<'de> Deserialize<'de> + 'static,
{
Expand Down Expand Up @@ -64,6 +77,7 @@ impl ArgMeta {
Ok(Box::new(resolved) as Box<dyn MagicAny>)
})
},
filter_trait_call_args,
}
}

Expand Down Expand Up @@ -144,6 +158,7 @@ impl NativeFunction {
pub fn new_method<Mode, This, Inputs, I>(
name: String,
function_meta: FunctionMeta,
arg_meta: ArgMeta,
implementation: I,
) -> Self
where
Expand Down
3 changes: 2 additions & 1 deletion turbopack/crates/turbo-tasks/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ use rustc_hash::FxHasher;
use crate::{
id::{FunctionId, TraitTypeId, ValueTypeId},
id_factory::IdFactory,
native_function::NativeFunction,
no_move_vec::NoMoveVec,
NativeFunction, TraitType, ValueType,
TraitType, ValueType,
};

type FxDashMap<K, V> = DashMap<K, V, BuildHasherDefault<FxHasher>>;
Expand Down
Loading