More ReadRef fixes using VcCellMode::raw_cell API

crates/turbo-tasks/src/completion.rs

@@ -36,7 +36,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()
     }

crates/turbo-tasks/src/manager.rs

@@ -1522,23 +1522,33 @@ pub struct CurrentCellRef {
     index: CellId,
 }
 
+type VcReadRepr<T> = <<T as VcValueType>::Read as VcRead<T>>::Repr;
+
 impl CurrentCellRef {
-    pub fn conditional_update_shared<
-        T: VcValueType + 'static,
-        F: FnOnce(Option<&T>) -> Option<T>,
-    >(
+    /// Updates the cell if the given `functor` returns a value.
+    pub fn conditional_update<T>(&self, functor: impl FnOnce(Option<&T>) -> Option<T>)
+    where
+        T: VcValueType,
+    {
+        self.conditional_update_with_shared_reference(|old_shared_reference| {
+            let old_ref = old_shared_reference
+                .and_then(|sr| sr.0.downcast_ref::<VcReadRepr<T>>())
+                .map(|content| <T::Read as VcRead<T>>::repr_to_value_ref(content));
+            let new_value = functor(old_ref)?;
+            Some(SharedReference::new(triomphe::Arc::new(
+                <T::Read as VcRead<T>>::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<SharedReference>,
     ) {
         let tt = turbo_tasks();
-        let content = tt
-            .read_own_task_cell(self.current_task, self.index)
-            .ok()
-            .and_then(|v| v.try_cast::<T>());
-        let update =
-            functor(content.as_deref().map(|content| {
-                <<T as VcValueType>::Read as VcRead<T>>::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.1 .0.as_ref()));
         if let Some(update) = update {
             tt.update_own_task_cell(
                 self.current_task,
@@ -1548,31 +1558,70 @@ impl CurrentCellRef {
         }
     }
 
-    pub fn compare_and_update_shared<T: PartialEq + VcValueType + 'static>(&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<T>(&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_value)
+        });
+    }
+
+    /// 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<T>(&self, new_shared_reference: SharedReference)
+    where
+        T: VcValueType + PartialEq,
+    {
+        fn extract_sr_value<T: VcValueType>(sr: &SharedReference) -> &T {
+            <T::Read as VcRead<T>>::repr_to_value_ref(
+                sr.0.downcast_ref::<VcReadRepr<T>>()
+                    .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_content)
+            Some(new_shared_reference)
         });
     }
 
-    pub fn update_shared<T: VcValueType + 'static>(&self, new_content: T) {
+    /// Unconditionally updates the content of the cell.
+    pub fn update<T>(&self, new_value: T)
+    where
+        T: VcValueType,
+    {
         let tt = turbo_tasks();
         tt.update_own_task_cell(
             self.current_task,
             self.index,
-            CellContent(Some(SharedReference::new(triomphe::Arc::new(new_content)))),
+            CellContent(Some(SharedReference::new(triomphe::Arc::new(
+                <T::Read as VcRead<T>>::value_to_repr(new_value),
+            )))),
         )
     }
 
-    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(TypedCellContent(_, CellContent(shared_ref_exp))) = content {
-            shared_ref_exp.as_ref().ne(&Some(&shared_ref))
+        let update = if let Some(TypedCellContent(_, CellContent(Some(shared_ref_exp)))) = content {
+            // pointer equality (not value equality)
+            shared_ref_exp != shared_ref
         } else {
             true
         };

crates/turbo-tasks/src/read_ref.rs

@@ -10,9 +10,9 @@ use turbo_tasks_hash::DeterministicHash;
 
 use crate::{
     debug::{ValueDebugFormat, ValueDebugFormatString},
-    macro_helpers::find_cell_by_type,
     trace::{TraceRawVcs, TraceRawVcsContext},
     triomphe_utils::unchecked_sidecast_triomphe_arc,
+    vc::VcCellMode,
     SharedReference, Vc, VcRead, VcValueType,
 };
 
@@ -244,14 +244,15 @@ where
     /// reference.
     pub fn cell(read_ref: ReadRef<T>) -> Vc<T> {
         let type_id = T::get_value_type_id();
-        let local_cell = find_cell_by_type(type_id);
         // SAFETY: `T` and `T::Read::Repr` must have equivalent memory representations,
         // guaranteed by the unsafe implementation of `VcValueType`.
-        local_cell.update_shared_reference(SharedReference::new(unsafe {
+        let value = unsafe {
             unchecked_sidecast_triomphe_arc::<T, <T::Read as VcRead<T>>::Repr>(read_ref.0)
-        }));
+        };
         Vc {
-            node: local_cell.into(),
+            node: <T::CellMode as VcCellMode<T>>::raw_cell(
+                SharedReference::new(value).typed(type_id),
+            ),
             _t: PhantomData,
         }
     }

crates/turbo-tasks/src/trait_ref.rs

@@ -102,7 +102,7 @@ where
         // See Safety clause above.
         let TypedSharedReference(ty, shared_ref) = trait_ref.shared_reference;
         let local_cell = find_cell_by_type(ty);
-        local_cell.update_shared_reference(shared_ref);
+        local_cell.update_with_shared_reference(shared_ref);
         let raw_vc: RawVc = local_cell.into();
         raw_vc.into()
     }

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, task::shared_reference::TypedSharedReference, RawVc, Vc};
 
-/// Trait that controls the behavior of `Vc::cell` on a value type basis.
+type VcReadTarget<T> = <<T as VcValueType>::Read as VcRead<T>>::Target;
+type VcReadRepr<T> = <<T as VcValueType>::Read as VcRead<T>>::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<T>
 where
     T: VcValueType,
 {
-    fn cell(value: <T::Read as VcRead<T>>::Target) -> Vc<T>;
+    /// Create a new cell.
+    fn cell(value: VcReadTarget<T>) -> Vc<T>;
+
+    /// 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: TypedSharedReference) -> RawVc;
 }
 
 /// Mode that always updates the cell's content.
@@ -22,14 +31,20 @@ impl<T> VcCellMode<T> for VcCellNewMode<T>
 where
     T: VcValueType,
 {
-    fn cell(inner: <T::Read as VcRead<T>>::Target) -> Vc<T> {
+    fn cell(inner: VcReadTarget<T>) -> Vc<T> {
         let cell = find_cell_by_type(T::get_value_type_id());
-        cell.update_shared(<T::Read as VcRead<T>>::target_to_repr(inner));
+        cell.update(<T::Read as VcRead<T>>::target_to_repr(inner));
         Vc {
             node: cell.into(),
             _t: PhantomData,
         }
     }
+
+    fn raw_cell(content: TypedSharedReference) -> RawVc {
+        let cell = find_cell_by_type(content.0);
+        cell.update_with_shared_reference(content.1);
+        cell.into()
+    }
 }
 
 /// Mode that compares the cell's content with the new value and only updates
@@ -43,12 +58,18 @@ where
     T: VcValueType,
     <T::Read as VcRead<T>>::Repr: PartialEq,
 {
-    fn cell(inner: <T::Read as VcRead<T>>::Target) -> Vc<T> {
+    fn cell(inner: VcReadTarget<T>) -> Vc<T> {
         let cell = find_cell_by_type(T::get_value_type_id());
-        cell.compare_and_update_shared(<T::Read as VcRead<T>>::target_to_repr(inner));
+        cell.compare_and_update(<T::Read as VcRead<T>>::target_to_repr(inner));
         Vc {
             node: cell.into(),
             _t: PhantomData,
         }
     }
+
+    fn raw_cell(content: TypedSharedReference) -> RawVc {
+        let cell = find_cell_by_type(content.0);
+        cell.compare_and_update_with_shared_reference::<VcReadRepr<T>>(content.1);
+        cell.into()
+    }
 }

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},

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,13 +69,21 @@ where
         value
     }
 
+    fn value_to_repr(value: T) -> Self::Repr {
+        value
+    }
+
     fn target_to_repr(target: Self::Target) -> T {
         target
     }
 
     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::<ManuallyDrop<T>, 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::<ManuallyDrop<&Self::Target>, &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::<ManuallyDrop<&Self::Repr>, &T>(&ManuallyDrop::new(repr))
+        }
+    }
 }
 
 pub struct ReadVcFuture<T, Cast = VcValueTypeCast<T>>

crates/turbopack-node/src/evaluate.rs

@@ -242,7 +242,7 @@ pub fn custom_evaluate(evaluate_context: impl EvaluateContext) -> Vc<JavaScriptE
     // We initialize the cell with a stream that is open, but has no values.
     // The first [compute_evaluate_stream] pipe call will pick up that stream.
     let (sender, receiver) = unbounded();
-    cell.update_shared(JavaScriptEvaluation(JavaScriptStream::new_open(
+    cell.update(JavaScriptEvaluation(JavaScriptStream::new_open(
         vec![],
         Box::new(receiver),
     )));
@@ -258,7 +258,7 @@ pub fn custom_evaluate(evaluate_context: impl EvaluateContext) -> Vc<JavaScriptE
                     // In cases when only [compute_evaluate_stream] is (re)executed, we need to
                     // update the old stream with a new value.
                     let (sender, receiver) = unbounded();
-                    cell.update_shared(JavaScriptEvaluation(JavaScriptStream::new_open(
+                    cell.update(JavaScriptEvaluation(JavaScriptStream::new_open(
                         vec![],
                         Box::new(receiver),
                     )));

crates/turbopack-node/src/render/render_proxy.rs

@@ -180,7 +180,7 @@ fn render_stream(options: RenderStreamOptions) -> Vc<RenderStream> {
     // 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<RenderStream> {
                     // 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
                 }
             }),

crates/turbopack-node/src/render/render_static.rs

@@ -231,7 +231,7 @@ fn render_stream(options: RenderStreamOptions) -> Vc<RenderStream> {
     // 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<RenderStream> {
                     // 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
                 }
             }),