diff --git a/crates/turbo-tasks/src/completion.rs b/crates/turbo-tasks/src/completion.rs index 36554d668ff543..a14f6e2272a35f 100644 --- a/crates/turbo-tasks/src/completion.rs +++ b/crates/turbo-tasks/src/completion.rs @@ -31,7 +31,7 @@ impl Completion { // only updates the cell when it is empty (Completion::cell opted-out of // that via `#[turbo_tasks::value(cell = "new")]`) let cell = turbo_tasks::macro_helpers::find_cell_by_type(*COMPLETION_VALUE_TYPE_ID); - cell.conditional_update_shared(|old| old.is_none().then_some(Completion)); + cell.conditional_update(|old| old.is_none().then_some(Completion)); let raw: RawVc = cell.into(); raw.into() } diff --git a/crates/turbo-tasks/src/manager.rs b/crates/turbo-tasks/src/manager.rs index 32acf158549095..1d5062c24f9daf 100644 --- a/crates/turbo-tasks/src/manager.rs +++ b/crates/turbo-tasks/src/manager.rs @@ -1539,30 +1539,51 @@ pub(crate) async fn read_task_cell( } } -#[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +/// A reference to a task's cell with methods that allow updating the contents +/// of the cell. +/// +/// Mutations should not outside of the task that that owns this cell. Doing so +/// is a logic error, and may lead to incorrect caching behavior. +#[derive(Clone, Copy, Serialize, Deserialize)] pub struct CurrentCellRef { current_task: TaskId, index: CellId, } +type VcReadRepr = <::Read as VcRead>::Repr; + impl CurrentCellRef { - pub fn conditional_update_shared< - T: VcValueType + 'static, - F: FnOnce(Option<&T>) -> Option, - >( + /// Updates the cell if the given `functor` returns a value. + pub fn conditional_update(&self, functor: impl FnOnce(Option<&T>) -> Option) + where + T: VcValueType, + { + self.conditional_update_with_shared_reference(|old_shared_reference| { + let old_ref = old_shared_reference + .and_then(|sr| sr.downcast_ref::>()) + .map(|content| >::repr_to_value_ref(content)); + let new_value = functor(old_ref)?; + Some(SharedReference::new( + Some(self.index.type_id), + triomphe::Arc::new(>::value_to_repr(new_value)), + )) + }) + } + + /// Updates the cell if the given `functor` returns a `SharedReference`. + pub fn conditional_update_with_shared_reference( &self, - functor: F, + functor: impl FnOnce(Option<&SharedReference>) -> Option, ) { let tt = turbo_tasks(); - let content = tt - .read_own_task_cell(self.current_task, self.index) - .ok() - .and_then(|v| v.try_cast::()); - let update = - functor(content.as_deref().map(|content| { - <::Read as VcRead>::target_to_value_ref(content) - })); + let cell_content = tt.read_own_task_cell(self.current_task, self.index).ok(); + let update = functor(cell_content.as_ref().and_then(|cc| cc.0.as_ref())); if let Some(update) = update { + debug_assert_eq!( + update.0, + Some(self.index.type_id), + "Cannot update cell with a SharedReference of a different type", + ); tt.update_own_task_cell( self.current_task, self.index, @@ -1574,18 +1595,54 @@ impl CurrentCellRef { } } - pub fn compare_and_update_shared(&self, new_content: T) { - self.conditional_update_shared(|old_content| { - if let Some(old_content) = old_content { - if PartialEq::eq(&new_content, old_content) { + /// Replace the current cell's content with `new_value` if the current + /// content is not equal by value with the existing content. + pub fn compare_and_update(&self, new_value: T) + where + T: PartialEq + VcValueType, + { + self.conditional_update(|old_value| { + if let Some(old_value) = old_value { + if old_value == &new_value { return None; } } - Some(new_content) + Some(new_value) }); } - pub fn update_shared(&self, new_content: T) { + /// Replace the current cell's content with `new_shared_reference` if the + /// current content is not equal by value with the existing content. + /// + /// If you already have a `SharedReference`, this is a faster version of + /// [`CurrentCellRef::compare_and_update`]. + pub fn compare_and_update_with_shared_reference(&self, new_shared_reference: SharedReference) + where + T: VcValueType + PartialEq, + { + fn extract_sr_value(sr: &SharedReference) -> &T { + >::repr_to_value_ref( + sr.downcast_ref::>() + .expect("cannot update SharedReference of different type"), + ) + } + self.conditional_update_with_shared_reference(|old_sr| { + if let Some(old_sr) = old_sr { + let old_value: &T = extract_sr_value(old_sr); + let new_value = extract_sr_value(&new_shared_reference); + if old_value == new_value { + return None; + } + } + Some(new_shared_reference) + }); + } + + /// Unconditionally updates the content of the cell. + pub fn update(&self, new_content: T) + where + T: VcValueType, + { let tt = turbo_tasks(); tt.update_own_task_cell( self.current_task, @@ -1597,10 +1654,11 @@ impl CurrentCellRef { ) } - pub fn update_shared_reference(&self, shared_ref: SharedReference) { + pub fn update_with_shared_reference(&self, shared_ref: SharedReference) { let tt = turbo_tasks(); let content = tt.read_own_task_cell(self.current_task, self.index).ok(); let update = if let Some(CellContent(Some(content))) = content { + // pointer equality (not value equality) content != shared_ref } else { true diff --git a/crates/turbo-tasks/src/raw_vc.rs b/crates/turbo-tasks/src/raw_vc.rs index 5249a373fb6f2c..8208ebabbe7eea 100644 --- a/crates/turbo-tasks/src/raw_vc.rs +++ b/crates/turbo-tasks/src/raw_vc.rs @@ -173,26 +173,15 @@ impl RawVc { /// See [`crate::Vc::resolve`]. pub(crate) async fn resolve(self) -> Result { - let tt = turbo_tasks(); - let mut current = self; - let mut notified = false; - loop { - match current { - RawVc::TaskOutput(task) => { - if !notified { - tt.notify_scheduled_tasks(); - notified = true; - } - current = read_task_output(&*tt, task, false).await?; - } - RawVc::TaskCell(_, _) => return Ok(current), - RawVc::LocalCell(_, _) => todo!(), - } - } + self.resolve_inner(/* strongly_consistent */ false).await } /// See [`crate::Vc::resolve_strongly_consistent`]. pub(crate) async fn resolve_strongly_consistent(self) -> Result { + self.resolve_inner(/* strongly_consistent */ true).await + } + + pub(crate) async fn resolve_inner(self, strongly_consistent: bool) -> Result { let tt = turbo_tasks(); let mut current = self; let mut notified = false; @@ -203,10 +192,18 @@ impl RawVc { tt.notify_scheduled_tasks(); notified = true; } - current = read_task_output(&*tt, task, true).await?; + current = read_task_output(&*tt, task, strongly_consistent).await?; } RawVc::TaskCell(_, _) => return Ok(current), - RawVc::LocalCell(_, _) => todo!(), + RawVc::LocalCell(execution_id, local_cell_id) => { + let shared_ref = read_local_cell(execution_id, local_cell_id); + let value_type = get_value_type( + shared_ref + .0 + .expect("local cells always include ValueTypeId"), + ); + return Ok((value_type.raw_cell)(shared_ref)); + } } } } diff --git a/crates/turbo-tasks/src/read_ref.rs b/crates/turbo-tasks/src/read_ref.rs index 3c1255a07cd12d..f5afe4a89b4eed 100644 --- a/crates/turbo-tasks/src/read_ref.rs +++ b/crates/turbo-tasks/src/read_ref.rs @@ -242,11 +242,9 @@ where /// Returns a new cell that points to the same value as the given /// reference. pub fn cell(read_ref: ReadRef) -> Vc { - let local_cell = find_cell_by_type(T::get_value_type_id()); - local_cell.update_shared_reference(SharedReference::new( - Some(T::get_value_type_id()), - read_ref.0, - )); + let type_id = T::get_value_type_id(); + let local_cell = find_cell_by_type(type_id); + local_cell.update_with_shared_reference(SharedReference::new(Some(type_id), read_ref.0)); Vc { node: local_cell.into(), _t: PhantomData, diff --git a/crates/turbo-tasks/src/task/shared_reference.rs b/crates/turbo-tasks/src/task/shared_reference.rs index 747f2477c98ca5..096bd91a84d655 100644 --- a/crates/turbo-tasks/src/task/shared_reference.rs +++ b/crates/turbo-tasks/src/task/shared_reference.rs @@ -32,6 +32,10 @@ impl SharedReference { Err(data) => Err(Self(self.0, data)), } } + + pub fn downcast_ref(&self) -> Option<&T> { + self.1.downcast_ref() + } } impl Hash for SharedReference { diff --git a/crates/turbo-tasks/src/trait_ref.rs b/crates/turbo-tasks/src/trait_ref.rs index d5eb54ecd83761..a587232cd3dea6 100644 --- a/crates/turbo-tasks/src/trait_ref.rs +++ b/crates/turbo-tasks/src/trait_ref.rs @@ -102,7 +102,7 @@ where let SharedReference(ty, _) = trait_ref.shared_reference; let ty = ty.unwrap(); let local_cell = find_cell_by_type(ty); - local_cell.update_shared_reference(trait_ref.shared_reference); + local_cell.update_with_shared_reference(trait_ref.shared_reference); let raw_vc: RawVc = local_cell.into(); raw_vc.into() } diff --git a/crates/turbo-tasks/src/value_type.rs b/crates/turbo-tasks/src/value_type.rs index d9d6e0246d8c22..9337aa16e5109c 100644 --- a/crates/turbo-tasks/src/value_type.rs +++ b/crates/turbo-tasks/src/value_type.rs @@ -16,10 +16,13 @@ use crate::{ id::{FunctionId, TraitTypeId}, magic_any::{AnyDeserializeSeed, MagicAny, MagicAnyDeserializeSeed, MagicAnySerializeSeed}, registry::{register_trait_type, register_value_type}, + vc::VcCellMode, + RawVc, SharedReference, VcValueType, }; type MagicSerializationFn = fn(&dyn MagicAny) -> &dyn erased_serde::Serialize; type AnySerializationFn = fn(&(dyn Any + Sync + Send)) -> &dyn erased_serde::Serialize; +type RawCellFactoryFn = fn(SharedReference) -> RawVc; // TODO this type need some refactoring when multiple languages are added to // turbo-task In this case a trait_method might be of a different function type. @@ -41,6 +44,17 @@ pub struct ValueType { /// Functors for serialization magic_serialization: Option<(MagicSerializationFn, MagicAnyDeserializeSeed)>, any_serialization: Option<(AnySerializationFn, AnyDeserializeSeed)>, + + /// An implementation of + /// [`VcCellMode::raw_cell`][crate::vc::cell_mode::VcCellMode::raw_cell]. + /// + /// Allows dynamically constructing a cell using the type id. Used inside of + /// [`RawVc`] where we have a type id, but not the concrete type `T` of + /// `Vc`. + /// + /// Because we allow resolving `Vc`, it's otherwise not possible + /// for `RawVc` to know what the appropriate `VcCellMode` is. + pub(crate) raw_cell: RawCellFactoryFn, } impl Hash for ValueType { @@ -88,19 +102,20 @@ pub fn any_as_serialize( impl ValueType { /// This is internally used by `#[turbo_tasks::value]` - pub fn new() -> Self { + pub fn new() -> Self { Self { name: std::any::type_name::().to_string(), traits: AutoSet::new(), trait_methods: AutoMap::new(), magic_serialization: None, any_serialization: None, + raw_cell: >::raw_cell, } } /// This is internally used by `#[turbo_tasks::value]` pub fn new_with_magic_serialization< - T: Debug + Eq + Hash + Serialize + for<'de> Deserialize<'de> + Send + Sync + 'static, + T: VcValueType + Debug + Eq + Hash + Serialize + for<'de> Deserialize<'de>, >() -> Self { Self { name: std::any::type_name::().to_string(), @@ -111,12 +126,13 @@ impl ValueType { MagicAnyDeserializeSeed::new::(), )), any_serialization: Some((any_as_serialize::, AnyDeserializeSeed::new::())), + raw_cell: >::raw_cell, } } /// This is internally used by `#[turbo_tasks::value]` pub fn new_with_any_serialization< - T: Any + Serialize + for<'de> Deserialize<'de> + Send + Sync + 'static, + T: VcValueType + Any + Serialize + for<'de> Deserialize<'de>, >() -> Self { Self { name: std::any::type_name::().to_string(), @@ -124,6 +140,7 @@ impl ValueType { trait_methods: AutoMap::new(), magic_serialization: None, any_serialization: Some((any_as_serialize::, AnyDeserializeSeed::new::())), + raw_cell: >::raw_cell, } } diff --git a/crates/turbo-tasks/src/vc/cell_mode.rs b/crates/turbo-tasks/src/vc/cell_mode.rs index 8c50017bfa263f..27f11f686c56f5 100644 --- a/crates/turbo-tasks/src/vc/cell_mode.rs +++ b/crates/turbo-tasks/src/vc/cell_mode.rs @@ -1,16 +1,25 @@ use std::marker::PhantomData; use super::{read::VcRead, traits::VcValueType}; -use crate::{manager::find_cell_by_type, Vc}; +use crate::{manager::find_cell_by_type, RawVc, SharedReference, Vc}; -/// Trait that controls the behavior of `Vc::cell` on a value type basis. +type VcReadTarget = <::Read as VcRead>::Target; +type VcReadRepr = <::Read as VcRead>::Repr; + +/// Trait that controls the behavior of [`Vc::cell`] based on the value type's +/// [`VcValueType::CellMode`]. /// /// This trait must remain sealed within this crate. pub trait VcCellMode where T: VcValueType, { - fn cell(value: >::Target) -> Vc; + /// Create a new cell. + fn cell(value: VcReadTarget) -> Vc; + + /// Create a type-erased `RawVc` cell given a pre-existing type-erased + /// `SharedReference`. In some cases, we will re-use the shared value. + fn raw_cell(value: SharedReference) -> RawVc; } /// Mode that always updates the cell's content. @@ -22,14 +31,20 @@ impl VcCellMode for VcCellNewMode where T: VcValueType, { - fn cell(inner: >::Target) -> Vc { + fn cell(inner: VcReadTarget) -> Vc { let cell = find_cell_by_type(T::get_value_type_id()); - cell.update_shared(>::target_to_repr(inner)); + cell.update(>::target_to_repr(inner)); Vc { node: cell.into(), _t: PhantomData, } } + + fn raw_cell(value: SharedReference) -> RawVc { + let cell = find_cell_by_type(T::get_value_type_id()); + cell.update_with_shared_reference(value); + cell.into() + } } /// Mode that compares the cell's content with the new value and only updates @@ -43,12 +58,18 @@ where T: VcValueType, >::Repr: PartialEq, { - fn cell(inner: >::Target) -> Vc { + fn cell(inner: VcReadTarget) -> Vc { let cell = find_cell_by_type(T::get_value_type_id()); - cell.compare_and_update_shared(>::target_to_repr(inner)); + cell.compare_and_update(>::target_to_repr(inner)); Vc { node: cell.into(), _t: PhantomData, } } + + fn raw_cell(value: SharedReference) -> RawVc { + let cell = find_cell_by_type(T::get_value_type_id()); + cell.compare_and_update_with_shared_reference::>(value); + cell.into() + } } diff --git a/crates/turbo-tasks/src/vc/mod.rs b/crates/turbo-tasks/src/vc/mod.rs index 08152541ed0246..8b529fb7030c99 100644 --- a/crates/turbo-tasks/src/vc/mod.rs +++ b/crates/turbo-tasks/src/vc/mod.rs @@ -16,10 +16,9 @@ use anyhow::Result; use auto_hash_map::AutoSet; use serde::{Deserialize, Serialize}; -use self::cell_mode::VcCellMode; pub use self::{ cast::{VcCast, VcValueTraitCast, VcValueTypeCast}, - cell_mode::{VcCellNewMode, VcCellSharedMode}, + cell_mode::{VcCellMode, VcCellNewMode, VcCellSharedMode}, default::ValueDefault, read::{ReadVcFuture, VcDefaultRead, VcRead, VcTransparentRead}, resolved::{ResolvedValue, ResolvedVc}, diff --git a/crates/turbo-tasks/src/vc/read.rs b/crates/turbo-tasks/src/vc/read.rs index e1c7c6034d2ce8..d97a0b885de38d 100644 --- a/crates/turbo-tasks/src/vc/read.rs +++ b/crates/turbo-tasks/src/vc/read.rs @@ -39,11 +39,17 @@ where /// Convert a reference to a value to a reference to the target type. fn value_to_target_ref(value: &T) -> &Self::Target; + /// Convert the value type to the repr. + fn value_to_repr(value: T) -> Self::Repr; + /// Convert the target type to the repr. fn target_to_repr(target: Self::Target) -> Self::Repr; /// Convert a reference to a target type to a reference to a value. fn target_to_value_ref(target: &Self::Target) -> &T; + + /// Convert a reference to a repr type to a reference to a value. + fn repr_to_value_ref(repr: &Self::Repr) -> &T; } /// Representation for standard `#[turbo_tasks::value]`, where a read return a @@ -63,6 +69,10 @@ where value } + fn value_to_repr(value: T) -> Self::Repr { + value + } + fn target_to_repr(target: Self::Target) -> T { target } @@ -70,6 +80,10 @@ where fn target_to_value_ref(target: &Self::Target) -> &T { target } + + fn repr_to_value_ref(repr: &Self::Repr) -> &T { + repr + } } /// Representation for `#[turbo_tasks::value(transparent)]` types, where reads @@ -98,6 +112,13 @@ where } } + fn value_to_repr(value: T) -> Self::Repr { + // Safety: see `Self::value_to_target` above. + unsafe { + std::mem::transmute_copy::, Self::Repr>(&ManuallyDrop::new(value)) + } + } + fn target_to_repr(target: Self::Target) -> Self::Repr { // Safety: see `Self::value_to_target` above. unsafe { @@ -113,6 +134,13 @@ where std::mem::transmute_copy::, &T>(&ManuallyDrop::new(target)) } } + + fn repr_to_value_ref(repr: &Self::Repr) -> &T { + // Safety: see `Self::value_to_target` above. + unsafe { + std::mem::transmute_copy::, &T>(&ManuallyDrop::new(repr)) + } + } } pub struct ReadVcFuture> diff --git a/crates/turbopack-node/src/evaluate.rs b/crates/turbopack-node/src/evaluate.rs index 505755b12c711d..91c0723c2a993b 100644 --- a/crates/turbopack-node/src/evaluate.rs +++ b/crates/turbopack-node/src/evaluate.rs @@ -242,7 +242,7 @@ pub fn custom_evaluate(evaluate_context: impl EvaluateContext) -> Vc Vc Vc { // We initialize the cell with a stream that is open, but has no values. // The first [render_stream_internal] pipe call will pick up that stream. let (sender, receiver) = unbounded(); - cell.update_shared(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); + cell.update(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); let initial = Mutex::new(Some(sender)); // run the evaluation as side effect @@ -194,7 +194,7 @@ fn render_stream(options: RenderStreamOptions) -> Vc { // In cases when only [render_stream_internal] is (re)executed, we need to // update the old stream with a new value. let (sender, receiver) = unbounded(); - cell.update_shared(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); + cell.update(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); sender } }), diff --git a/crates/turbopack-node/src/render/render_static.rs b/crates/turbopack-node/src/render/render_static.rs index 05e3a161f63799..5835cd634a90a2 100644 --- a/crates/turbopack-node/src/render/render_static.rs +++ b/crates/turbopack-node/src/render/render_static.rs @@ -231,7 +231,7 @@ fn render_stream(options: RenderStreamOptions) -> Vc { // We initialize the cell with a stream that is open, but has no values. // The first [render_stream_internal] pipe call will pick up that stream. let (sender, receiver) = unbounded(); - cell.update_shared(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); + cell.update(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); let initial = Mutex::new(Some(sender)); // run the evaluation as side effect @@ -245,7 +245,7 @@ fn render_stream(options: RenderStreamOptions) -> Vc { // In cases when only [render_stream_internal] is (re)executed, we need to // update the old stream with a new value. let (sender, receiver) = unbounded(); - cell.update_shared(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); + cell.update(RenderStream(Stream::new_open(vec![], Box::new(receiver)))); sender } }),