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

Respect VcCellMode in TraitRef::cell #8870

Closed
wants to merge 1 commit into from
Closed
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
144 changes: 144 additions & 0 deletions crates/turbo-tasks-memory/tests/trait_ref_cell_mode.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#![feature(arbitrary_self_types)]

use anyhow::Result;
use turbo_tasks::{IntoTraitRef, State, TraitRef, Upcast, Vc};
use turbo_tasks_testing::{register, run, Registration};

static REGISTRATION: Registration = register!();

// Test that with `cell = "shared"`, the cell will be re-used as long as the
// value is equal.
#[tokio::test]
async fn test_trait_ref_shared_cell_mode() {
run(&REGISTRATION, async {
let input = CellIdSelector {
value: 42,
cell_idx: State::new(0),
}
.cell();

// create the task and compute it
let counter_value_vc = shared_value_from_input(input);
let trait_ref_a = counter_value_vc.into_trait_ref().await.unwrap();

// invalidate the task, and pick a different cell id for the next execution
input.await.unwrap().cell_idx.set_unconditionally(1);

// recompute the task
let trait_ref_b = counter_value_vc.into_trait_ref().await.unwrap();

for trait_ref in [&trait_ref_a, &trait_ref_b] {
assert_eq!(
*TraitRef::cell(trait_ref.clone()).get_value().await.unwrap(),
42
);
}

// because we're using `cell = "shared"`, these trait refs must use the same
// underlying Arc/SharedRef (by identity)
assert!(TraitRef::ptr_eq(&trait_ref_a, &trait_ref_b));
})
.await
}

// Test that with `cell = "new"`, the cell will is never re-used, even if the
// value is equal.
#[tokio::test]
async fn test_trait_ref_new_cell_mode() {
run(&REGISTRATION, async {
let input = CellIdSelector {
value: 42,
cell_idx: State::new(0),
}
.cell();

// create the task and compute it
let counter_value_vc = new_value_from_input(input);
let trait_ref_a = counter_value_vc.into_trait_ref().await.unwrap();

// invalidate the task, and pick a different cell id for the next execution
input.await.unwrap().cell_idx.set_unconditionally(1);

// recompute the task
let trait_ref_b = counter_value_vc.into_trait_ref().await.unwrap();

for trait_ref in [&trait_ref_a, &trait_ref_b] {
assert_eq!(
*TraitRef::cell(trait_ref.clone()).get_value().await.unwrap(),
42
);
}

// because we're using `cell = "new"`, these trait refs must use different
// underlying Arc/SharedRefs (by identity)
assert!(!TraitRef::ptr_eq(&trait_ref_a, &trait_ref_b));
})
.await
}

#[turbo_tasks::value_trait]
trait ValueTrait {
fn get_value(&self) -> Vc<usize>;
}

#[turbo_tasks::value(transparent, cell = "shared")]
struct SharedValue(usize);

#[turbo_tasks::value(transparent, cell = "new")]
struct NewValue(usize);

#[turbo_tasks::value_impl]
impl ValueTrait for SharedValue {
#[turbo_tasks::function]
fn get_value(&self) -> Vc<usize> {
Vc::cell(self.0)
}
}

#[turbo_tasks::value_impl]
impl ValueTrait for NewValue {
#[turbo_tasks::function]
fn get_value(&self) -> Vc<usize> {
Vc::cell(self.0)
}
}

#[turbo_tasks::value]
struct CellIdSelector {
value: usize,
cell_idx: State<usize>,
}

async fn value_from_input<T>(
input: Vc<CellIdSelector>,
mut cell_fn: impl FnMut(usize) -> Vc<T>,
) -> Result<Vc<Box<dyn ValueTrait>>>
where
T: ValueTrait + Upcast<Box<dyn ValueTrait>>,
{
let input = input.await?;

// create multiple cells so that we can pick from them, simulating a function
// with non-deterministic ordering that returns a "random" cell that happens to
// contain the same value
let mut upcast_vcs = Vec::new();
for _idx in 0..2 {
upcast_vcs.push(Vc::upcast((cell_fn)(input.value)));
}

// pick a different cell idx upon each invalidation/execution
let picked_vc = upcast_vcs[*input.cell_idx.get()];

// round-trip through `TraitRef::cell`
Ok(TraitRef::cell(picked_vc.into_trait_ref().await?))
}

#[turbo_tasks::function]
async fn shared_value_from_input(input: Vc<CellIdSelector>) -> Result<Vc<Box<dyn ValueTrait>>> {
value_from_input::<SharedValue>(input, Vc::<SharedValue>::cell).await
}

#[turbo_tasks::function]
async fn new_value_from_input(input: Vc<CellIdSelector>) -> Result<Vc<Box<dyn ValueTrait>>> {
value_from_input::<NewValue>(input, Vc::<NewValue>::cell).await
}
19 changes: 11 additions & 8 deletions crates/turbo-tasks/src/trait_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use anyhow::Result;
use serde::{Deserialize, Serialize};

use crate::{
manager::find_cell_by_type,
registry::get_value_type,
task::shared_reference::TypedSharedReference,
vc::{cast::VcCast, ReadVcFuture, VcValueTraitCast},
RawVc, Vc, VcValueTrait,
Vc, VcValueTrait,
};

/// Similar to a [`ReadRef<T>`][crate::ReadRef], but contains a value trait
Expand Down Expand Up @@ -90,6 +90,10 @@ where
_t: PhantomData,
}
}

pub fn ptr_eq(this: &Self, other: &Self) -> bool {
triomphe::Arc::ptr_eq(&this.shared_reference.1 .0, &other.shared_reference.1 .0)
}
}

impl<T> TraitRef<T>
Expand All @@ -99,12 +103,11 @@ where
/// Returns a new cell that points to a value that implements the value
/// trait `T`.
pub fn cell(trait_ref: TraitRef<T>) -> Vc<T> {
// See Safety clause above.
let TypedSharedReference(ty, shared_ref) = trait_ref.shared_reference;
let local_cell = find_cell_by_type(ty);
local_cell.update_with_shared_reference(shared_ref);
let raw_vc: RawVc = local_cell.into();
raw_vc.into()
let TraitRef {
shared_reference, ..
} = trait_ref;
let value_type = get_value_type(shared_reference.0);
(value_type.raw_cell)(shared_reference).into()
}
}

Expand Down
Loading