Skip to content

Commit

Permalink
perf(turbo-tasks): Add a 'local' option to `#[turbo_tasks::function(.…
Browse files Browse the repository at this point in the history
….)]`
  • Loading branch information
bgw committed Jan 24, 2025
1 parent 47a0cd2 commit 0c17996
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 69 deletions.
1 change: 1 addition & 0 deletions turbopack/crates/turbo-tasks-backend/tests/local_tasks.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: unexpected token, expected one of: "fs", "network", "operation", or "local_cells"
error: unexpected token, expected one of: "fs", "network", "operation", "local", or "local_cells"
--> tests/function/fail_attribute_invalid_args.rs:9:25
|
9 | #[turbo_tasks::function(invalid_argument)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: unexpected token, expected one of: "fs", "network", "operation", or "local_cells"
error: unexpected token, expected one of: "fs", "network", "operation", "local", or "local_cells"
--> tests/function/fail_attribute_invalid_args_inherent_impl.rs:14:29
|
14 | #[turbo_tasks::function(invalid_argument)]
Expand Down
57 changes: 28 additions & 29 deletions turbopack/crates/turbo-tasks-macros/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub struct TurboFn<'a> {
/// Should we return `OperationVc` and require that all arguments are `NonLocalValue`s?
operation: bool,
/// Should this function use `TaskPersistence::LocalCells`?
local_cells: bool,
local: bool,
}

#[derive(Debug)]
Expand Down Expand Up @@ -274,7 +274,7 @@ impl TurboFn<'_> {
this,
inputs,
operation: args.operation.is_some(),
local_cells: args.local_cells.is_some(),
local: args.local.is_some(),
inline_ident,
})
}
Expand Down Expand Up @@ -479,9 +479,9 @@ impl TurboFn<'_> {
}

pub fn persistence(&self) -> impl ToTokens {
if self.local_cells {
if self.local {
quote! {
turbo_tasks::TaskPersistence::LocalCells
turbo_tasks::TaskPersistence::Local
}
} else {
quote! {
Expand All @@ -491,9 +491,9 @@ impl TurboFn<'_> {
}

pub fn persistence_with_this(&self) -> impl ToTokens {
if self.local_cells {
if self.local {
quote! {
turbo_tasks::TaskPersistence::LocalCells
turbo_tasks::TaskPersistence::Local
}
} else {
quote! {
Expand Down Expand Up @@ -703,6 +703,10 @@ pub struct FunctionArguments {
///
/// If there is an error due to this option being set, it should be reported to this span.
operation: Option<Span>,
/// Does not run the function as a real task, and instead runs it inside the parent task using
/// task-local state. The function call itself will not be cached, but cells will be created on
/// the parent task.
pub local: Option<Span>,
/// Changes the behavior of `Vc::cell` to create local cells that are not cached across task
/// executions. Cells can be converted to their non-local versions by calling `Vc::resolve`.
///
Expand Down Expand Up @@ -732,23 +736,29 @@ impl Parse for FunctionArguments {
("operation", Meta::Path(_)) => {
parsed_args.operation = Some(meta.span());
}
("local", Meta::Path(_)) => {
parsed_args.local = Some(meta.span());
}
("local_cells", Meta::Path(_)) => {
let span = Some(meta.span());
parsed_args.local_cells = span;
parsed_args.local_cells = Some(meta.span());
}
(_, meta) => {
return Err(syn::Error::new_spanned(
meta,
"unexpected token, expected one of: \"fs\", \"network\", \"operation\", \
or \"local_cells\"",
\"local\", or \"local_cells\"",
))
}
}
}
if let (Some(_), Some(span)) = (parsed_args.local_cells, parsed_args.operation) {
if let (Some(_), Some(span)) = (
parsed_args.local.or(parsed_args.local_cells),
parsed_args.operation,
) {
return Err(syn::Error::new(
span,
"\"operation\" is mutually exclusive with the \"local_cells\" option",
"\"operation\" is mutually exclusive with the \"local\" and \"local_cells\" \
options",
));
}
Ok(parsed_args)
Expand Down Expand Up @@ -1023,27 +1033,14 @@ impl DefinitionContext {

#[derive(Debug)]
pub struct NativeFn {
function_path_string: String,
function_path: ExprPath,
is_method: bool,
local_cells: bool,
pub function_path_string: String,
pub function_path: ExprPath,
pub is_method: bool,
pub local: bool,
pub local_cells: bool,
}

impl NativeFn {
pub fn new(
function_path_string: &str,
function_path: &ExprPath,
is_method: bool,
local_cells: bool,
) -> NativeFn {
NativeFn {
function_path_string: function_path_string.to_owned(),
function_path: function_path.clone(),
is_method,
local_cells,
}
}

pub fn ty(&self) -> Type {
parse_quote! { turbo_tasks::NativeFunction }
}
Expand All @@ -1053,6 +1050,7 @@ impl NativeFn {
function_path_string,
function_path,
is_method,
local,
local_cells,
} = self;

Expand All @@ -1068,6 +1066,7 @@ impl NativeFn {
turbo_tasks::NativeFunction::#constructor(
#function_path_string.to_owned(),
turbo_tasks::FunctionMeta {
local: #local,
local_cells: #local_cells,
},
#function_path,
Expand Down
12 changes: 7 additions & 5 deletions turbopack/crates/turbo-tasks-macros/src/function_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
let args = syn::parse::<FunctionArguments>(args)
.inspect_err(|err| errors.push(err.to_compile_error()))
.unwrap_or_default();
let local = args.local.is_some();
let local_cells = args.local_cells.is_some();

let Some(turbo_fn) = TurboFn::new(&sig, DefinitionContext::NakedFn, args) else {
Expand All @@ -54,12 +55,13 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
let (inline_signature, inline_block) = turbo_fn.inline_signature_and_block(&block);
let inline_attrs = filter_inline_attributes(&attrs[..]);

let native_fn = NativeFn::new(
&ident.to_string(),
&parse_quote! { #inline_function_ident },
turbo_fn.is_method(),
let native_fn = NativeFn {
function_path_string: ident.to_string(),
function_path: parse_quote! { #inline_function_ident },
is_method: turbo_fn.is_method(),
local,
local_cells,
);
};
let native_function_ident = get_native_function_ident(ident);
let native_function_ty = native_fn.ty();
let native_function_def = native_fn.definition();
Expand Down
24 changes: 14 additions & 10 deletions turbopack/crates/turbo-tasks-macros/src/value_impl_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
let func_args = func_args
.inspect_err(|err| errors.push(err.to_compile_error()))
.unwrap_or_default();
let local = func_args.local.is_some();
let local_cells = func_args.local_cells.is_some();

let Some(turbo_fn) =
Expand All @@ -135,12 +136,13 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
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::new(
&format!("{ty}::{ident}", ty = ty.to_token_stream()),
&parse_quote! { <#ty>::#inline_function_ident },
turbo_fn.is_method(),
let native_fn = NativeFn {
function_path_string: format!("{ty}::{ident}", ty = ty.to_token_stream()),
function_path: parse_quote! { <#ty>::#inline_function_ident },
is_method: turbo_fn.is_method(),
local,
local_cells,
);
};

let native_function_ident = get_inherent_impl_function_ident(ty_ident, ident);
let native_function_ty = native_fn.ty();
Expand Down Expand Up @@ -221,6 +223,7 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
let func_args = func_args
.inspect_err(|err| errors.push(err.to_compile_error()))
.unwrap_or_default();
let local = func_args.local.is_some();
let local_cells = func_args.local_cells.is_some();

let Some(turbo_fn) =
Expand All @@ -239,18 +242,19 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
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::new(
&format!(
let native_fn = NativeFn {
function_path_string: format!(
"<{ty} as {trait_path}>::{ident}",
ty = ty.to_token_stream(),
trait_path = trait_path.to_token_stream()
),
&parse_quote! {
function_path: parse_quote! {
<#ty as #inline_extension_trait_ident>::#inline_function_ident
},
turbo_fn.is_method(),
is_method: turbo_fn.is_method(),
local,
local_cells,
);
};

let native_function_ident =
get_trait_impl_function_ident(ty_ident, &trait_ident, ident);
Expand Down
17 changes: 9 additions & 8 deletions turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,19 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
let (inline_signature, inline_block) = turbo_fn.inline_signature_and_block(default);
let inline_attrs = filter_inline_attributes(&attrs[..]);

let native_function = NativeFn::new(
&format!("{trait_ident}::{ident}"),
&parse_quote! {
let native_function = NativeFn {
function_path_string: format!("{trait_ident}::{ident}"),
function_path: parse_quote! {
<Box<dyn #trait_ident> as #inline_extension_trait_ident>::#inline_function_ident
},
turbo_fn.is_method(),
// `inline_cells` is currently unsupported here because:
is_method: turbo_fn.is_method(),
// `local` and `local_cells` are currently unsupported here because:
// - The `#[turbo_tasks::function]` macro needs to be present for us to read this
// argument.
// argument. (This could be fixed)
// - This only makes sense when a default implementation is present.
false,
);
local: false,
local_cells: false,
};

let native_function_ident = get_trait_default_impl_function_ident(trait_ident, ident);
let native_function_ty = native_function.ty();
Expand Down
1 change: 1 addition & 0 deletions turbopack/crates/turbo-tasks-memory/tests/local_tasks.rs
34 changes: 34 additions & 0 deletions turbopack/crates/turbo-tasks-testing/tests/local_tasks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![feature(arbitrary_self_types)]
#![feature(arbitrary_self_types_pointers)]
#![allow(clippy::needless_return)] // tokio macro-generated code doesn't respect this

use anyhow::Result;
use turbo_tasks::{test_helpers::current_task_for_testing, Vc};
use turbo_tasks_testing::{register, run, Registration};

static REGISTRATION: Registration = register!();

#[tokio::test]
async fn test_local_task_id() -> Result<()> {
run(&REGISTRATION, || async {
let local_vc = get_local_task_id();
assert!(local_vc.is_local());
assert_eq!(*local_vc.await.unwrap(), *current_task_for_testing());

let non_local_vc = get_non_local_task_id();
assert!(!non_local_vc.is_local());
assert_ne!(*non_local_vc.await.unwrap(), *current_task_for_testing());
Ok(())
})
.await
}

#[turbo_tasks::function(local)]
fn get_local_task_id() -> Vc<u32> {
Vc::cell(*current_task_for_testing())
}

#[turbo_tasks::function]
fn get_non_local_task_id() -> Vc<u32> {
Vc::cell(*current_task_for_testing())
}
16 changes: 7 additions & 9 deletions turbopack/crates/turbo-tasks/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,19 +313,15 @@ pub enum TaskPersistence {
/// [`TransientInstance`][crate::value::TransientInstance].
Transient,

/// Tasks that are persisted only for the lifetime of the nearest non-`LocalCells` parent
/// caller.
/// Tasks that are persisted only for the lifetime of the nearest non-`Local` parent caller.
///
/// This task does not have a unique task id, and is not shared with the backend. Instead it
/// uses the parent task's id.
///
/// Cells are allocated onto a temporary arena by default. Resolved cells inside a local task
/// are allocated into the parent task's cells.
///
/// This is useful for functions that have a low cache hit rate. Those functions could be
/// converted to non-task functions, but that would break their function signature. This
/// provides a mechanism for skipping caching without changing the function signature.
LocalCells,
Local,
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -588,19 +584,21 @@ impl<B: Backend + 'static> TurboTasks<B> {
arg: Box<dyn MagicAny>,
persistence: TaskPersistence,
) -> RawVc {
let task_type = CachedTaskType { fn_type, this, arg };
match persistence {
TaskPersistence::LocalCells => {
todo!("bgw: local tasks");
TaskPersistence::Local => {
let task_type = LocalTaskType::Native { fn_type, this, arg };
self.schedule_local_task(task_type, persistence)
}
TaskPersistence::Transient => {
let task_type = CachedTaskType { fn_type, this, arg };
RawVc::TaskOutput(self.backend.get_or_create_transient_task(
task_type,
current_task("turbo_function calls"),
self,
))
}
TaskPersistence::Persistent => {
let task_type = CachedTaskType { fn_type, this, arg };
RawVc::TaskOutput(self.backend.get_or_create_persistent_task(
task_type,
current_task("turbo_function calls"),
Expand Down
10 changes: 7 additions & 3 deletions turbopack/crates/turbo-tasks/src/native_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ impl ArgMeta {

#[derive(Debug)]
pub struct FunctionMeta {
/// Does not run the function as a task, and instead runs it inside the parent task using
/// task-local state. The function call itself will not be cached, but cells will be created on
/// the parent task.
pub local: bool,
/// Changes the behavior of `Vc::cell` to create local cells that are not
/// cached across task executions. Cells can be converted to their non-local
/// versions by calling `Vc::resolve`.
Expand Down Expand Up @@ -175,7 +179,7 @@ impl NativeFunction {
transient = true,
)
}
TaskPersistence::LocalCells => {
TaskPersistence::Local => {
tracing::trace_span!(
"turbo_tasks::function",
name = self.name.as_str(),
Expand All @@ -197,11 +201,11 @@ impl NativeFunction {
transient = true,
)
}
TaskPersistence::LocalCells => {
TaskPersistence::Local => {
tracing::trace_span!(
"turbo_tasks::resolve_call",
name = self.name.as_str(),
local_cells = true,
local = true,
)
}
}
Expand Down
Loading

0 comments on commit 0c17996

Please sign in to comment.