From ce48d7cb6afe15966569c1a417af6724ec24bbd8 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 13 Aug 2024 07:46:33 +0200 Subject: [PATCH] [Turbopack] serialization fixups (#68715) ### What? Fix some incorrect serialization code that has never been tested Need to use fork of triomphe to convert `Box` to `Arc`. remove unused initializate function from backend --- Cargo.lock | 15 ++++++++++++--- Cargo.toml | 2 +- .../turbo-tasks-macros/src/value_trait_macro.rs | 4 ++-- turbopack/crates/turbo-tasks/Cargo.toml | 2 +- turbopack/crates/turbo-tasks/src/backend.rs | 11 ++++------- turbopack/crates/turbo-tasks/src/id.rs | 2 +- turbopack/crates/turbo-tasks/src/manager.rs | 3 +-- .../turbo-tasks/src/task/shared_reference.rs | 3 ++- 8 files changed, 24 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1fb32520e7cb..865c4cbe45493 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2676,7 +2676,7 @@ dependencies = [ "once_cell", "phf 0.11.2", "rustc-hash", - "triomphe", + "triomphe 0.1.13", ] [[package]] @@ -8228,6 +8228,16 @@ dependencies = [ "tracing-serde", ] +[[package]] +name = "triomphe" +version = "0.1.12" +source = "git+https://github.com/sokra/triomphe?branch=sokra/unstable#9c591aba443d01a9f525fe9b4a671359d1b455d2" +dependencies = [ + "serde", + "stable_deref_trait", + "unsize", +] + [[package]] name = "triomphe" version = "0.1.13" @@ -8236,7 +8246,6 @@ checksum = "e6631e42e10b40c0690bf92f404ebcfe6e1fdb480391d15f17cc8e96eeed5369" dependencies = [ "serde", "stable_deref_trait", - "unsize", ] [[package]] @@ -8366,7 +8375,7 @@ dependencies = [ "thiserror", "tokio", "tracing", - "triomphe", + "triomphe 0.1.12", "turbo-tasks-build", "turbo-tasks-hash", "turbo-tasks-macros", diff --git a/Cargo.toml b/Cargo.toml index a7e115fecfaae..9c69dcbeb75df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -207,7 +207,7 @@ tokio = "1.25.0" tokio-util = { version = "0.7.7", features = ["io"] } tracing = "0.1.37" tracing-subscriber = "0.3.16" -triomphe = "0.1.13" +triomphe = { git = "https://github.com/sokra/triomphe", branch = "sokra/unstable" } unicode-segmentation = "1.10.1" unsize = "1.1.0" url = "2.2.2" diff --git a/turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs b/turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs index 03dc1405b6124..2a66f757f5490 100644 --- a/turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs +++ b/turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs @@ -133,7 +133,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream { }); trait_methods.push(quote! { - trait_type.register_default_trait_method::<(#(#arg_types),*)>(stringify!(#ident).into(), *#native_function_id_ident); + trait_type.register_default_trait_method::<(#(#arg_types,)*)>(stringify!(#ident).into(), *#native_function_id_ident); }); native_functions.push(quote! { @@ -171,7 +171,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream { Some(turbo_fn.static_block(&native_function_id_ident)) } else { trait_methods.push(quote! { - trait_type.register_trait_method::<(#(#arg_types),*)>(stringify!(#ident).into()); + trait_type.register_trait_method::<(#(#arg_types,)*)>(stringify!(#ident).into()); }); None }; diff --git a/turbopack/crates/turbo-tasks/Cargo.toml b/turbopack/crates/turbo-tasks/Cargo.toml index e080abbd685bc..d663d0e107010 100644 --- a/turbopack/crates/turbo-tasks/Cargo.toml +++ b/turbopack/crates/turbo-tasks/Cargo.toml @@ -40,7 +40,7 @@ serde_regex = "1.1.0" thiserror = { workspace = true } tokio = { workspace = true, features = ["full"] } tracing = { workspace = true } -triomphe = { workspace = true, features = ["unsize"] } +triomphe = { workspace = true, features = ["unsize", "unstable"] } turbo-tasks-hash = { workspace = true } turbo-tasks-macros = { workspace = true } turbo-tasks-malloc = { workspace = true } diff --git a/turbopack/crates/turbo-tasks/src/backend.rs b/turbopack/crates/turbo-tasks/src/backend.rs index 10f208df7cae4..25de8bb8a867f 100644 --- a/turbopack/crates/turbo-tasks/src/backend.rs +++ b/turbopack/crates/turbo-tasks/src/backend.rs @@ -23,8 +23,8 @@ use crate::{ task::shared_reference::TypedSharedReference, trait_helpers::{get_trait_method, has_trait, traits}, triomphe_utils::unchecked_sidecast_triomphe_arc, - FunctionId, RawVc, ReadRef, SharedReference, TaskId, TaskIdProvider, TaskIdSet, TraitRef, - TraitTypeId, ValueTypeId, VcRead, VcValueTrait, VcValueType, + FunctionId, RawVc, ReadRef, SharedReference, TaskId, TaskIdSet, TraitRef, TraitTypeId, + ValueTypeId, VcRead, VcValueTrait, VcValueType, }; pub enum TaskType { @@ -188,7 +188,7 @@ mod ser { s.serialize_element::(&1)?; s.serialize_element(&FunctionAndArg::Borrowed { fn_type: *fn_type, - arg, + arg: &**arg, })?; s.serialize_element(this)?; s.end() @@ -207,7 +207,7 @@ mod ser { let arg = if let Some(method) = registry::get_trait(*trait_type).methods.get(method_name) { - method.arg_serializer.as_serialize(arg) + method.arg_serializer.as_serialize(&**arg) } else { return Err(serde::ser::Error::custom("Method not found")); }; @@ -428,9 +428,6 @@ impl TryFrom for SharedReference { pub type TaskCollectiblesMap = AutoMap, 1>; pub trait Backend: Sync + Send { - #[allow(unused_variables)] - fn initialize(&mut self, task_id_provider: &dyn TaskIdProvider) {} - #[allow(unused_variables)] fn startup(&self, turbo_tasks: &dyn TurboTasksBackendApi) {} diff --git a/turbopack/crates/turbo-tasks/src/id.rs b/turbopack/crates/turbo-tasks/src/id.rs index 6eefce9506067..4312e3ca184e8 100644 --- a/turbopack/crates/turbo-tasks/src/id.rs +++ b/turbopack/crates/turbo-tasks/src/id.rs @@ -176,6 +176,6 @@ impl<'de> Deserialize<'de> for TaskId { } } - deserializer.deserialize_u64(V) + deserializer.deserialize_u32(V) } } diff --git a/turbopack/crates/turbo-tasks/src/manager.rs b/turbopack/crates/turbo-tasks/src/manager.rs index c7fd82d1bb370..cd453ced826de 100644 --- a/turbopack/crates/turbo-tasks/src/manager.rs +++ b/turbopack/crates/turbo-tasks/src/manager.rs @@ -316,9 +316,8 @@ impl TurboTasks { // that should be safe as long tasks can't outlife turbo task // so we probably want to make sure that all tasks are joined // when trying to drop turbo tasks - pub fn new(mut backend: B) -> Arc { + pub fn new(backend: B) -> Arc { let task_id_factory = IdFactoryWithReuse::new(); - backend.initialize(&task_id_factory); let this = Arc::new_cyclic(|this| Self { this: this.clone(), backend, diff --git a/turbopack/crates/turbo-tasks/src/task/shared_reference.rs b/turbopack/crates/turbo-tasks/src/task/shared_reference.rs index bb1e94dda1d03..cef099af92569 100644 --- a/turbopack/crates/turbo-tasks/src/task/shared_reference.rs +++ b/turbopack/crates/turbo-tasks/src/task/shared_reference.rs @@ -149,7 +149,8 @@ impl<'de> Deserialize<'de> for TypedSharedReference { if let Some(seed) = registry::get_value_type(ty).get_any_deserialize_seed() { if let Some(value) = seq.next_element_seed(seed)? { - Ok(TypedSharedReference(ty, SharedReference::new(value.into()))) + let arc = triomphe::Arc::::from(value); + Ok(TypedSharedReference(ty, SharedReference(arc))) } else { Err(serde::de::Error::invalid_length( 1,