Skip to content

Commit

Permalink
perf(turbo-tasks): Add a synchronous try_sidecast method to ResolvedVc (
Browse files Browse the repository at this point in the history
#74844)

If we have a `ResolvedVc`, we don't actually need to go through `Vc`'s much-less-efficient codepath (which reads cells, even if the `Vc` is resolved) in order to perform a sidecast.

This has the benefit of removing the chance of sidecast returning an error while reading and makes the function synchronous.

This does not update callsites, that can come later. We can do similar optimizations with `try_downcast` in a future PR.

I don't think this is an actual hot-path in our code, but it unblocks some synchronous graph traversal stuff that @wbinnssmith is working on.
  • Loading branch information
bgw authored Jan 15, 2025
1 parent d74d9c4 commit 4078bf9
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 4 deletions.
32 changes: 32 additions & 0 deletions turbopack/crates/turbo-tasks-testing/tests/resolved_vc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,35 @@ async fn test_into_future() -> Result<()> {
})
.await
}

#[tokio::test]
async fn test_sidecast() -> Result<()> {
run(&REGISTRATION, || async {
let concrete_value = ImplementsAandB.resolved_cell();
let as_a = ResolvedVc::upcast::<Box<dyn TraitA>>(concrete_value);
let as_b = ResolvedVc::try_sidecast_sync::<Box<dyn TraitB>>(as_a);
assert!(as_b.is_some());
let as_c = ResolvedVc::try_sidecast_sync::<Box<dyn TraitC>>(as_a);
assert!(as_c.is_none());
Ok(())
})
.await
}

#[turbo_tasks::value_trait]
trait TraitA {}

#[turbo_tasks::value_trait]
trait TraitB {}

#[turbo_tasks::value_trait]
trait TraitC {}

#[turbo_tasks::value]
struct ImplementsAandB;

#[turbo_tasks::value_impl]
impl TraitA for ImplementsAandB {}

#[turbo_tasks::value_impl]
impl TraitB for ImplementsAandB {}
11 changes: 11 additions & 0 deletions turbopack/crates/turbo-tasks/src/raw_vc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,17 @@ impl RawVc {
}
}
}

/// For a type that's already resolved, synchronously check if it implements a trait using the
/// type information in `RawVc::TaskCell` (we don't actualy need to read the cell!).
pub(crate) fn resolved_has_trait(&self, trait_id: TraitTypeId) -> bool {
match self {
RawVc::TaskCell(_task_id, cell_id) => {
get_value_type(cell_id.type_id).has_trait(&trait_id)
}
_ => unreachable!("resolved_has_trait must be called with a RawVc::TaskCell"),
}
}
}

impl CollectiblesSource for RawVc {
Expand Down
32 changes: 28 additions & 4 deletions turbopack/crates/turbo-tasks/src/vc/resolved.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{
fmt::Debug,
future::IntoFuture,
hash::{Hash, Hasher},
marker::PhantomData,
ops::Deref,
};

Expand Down Expand Up @@ -215,10 +216,33 @@ where
where
K: VcValueTrait + ?Sized,
{
// must be async, as we must read the cell to determine the type
Ok(Vc::try_resolve_sidecast(this.node)
.await?
.map(|node| ResolvedVc { node }))
// TODO: Expose a synchronous API instead of this async one that returns `Result<Option<_>>`
Ok(Self::try_sidecast_sync(this))
}

/// Attempts to sidecast the given `ResolvedVc<Box<dyn T>>` to a `ResolvedVc<Box<dyn K>>`.
///
/// Returns `None` if the underlying value type does not implement `K`.
///
/// **Note:** if the trait `T` is required to implement `K`, use [`ResolvedVc::upcast`] instead.
/// This provides stronger guarantees, removing the need for a [`Result`] return type.
///
/// See also: [`Vc::try_resolve_sidecast`].
pub fn try_sidecast_sync<K>(this: Self) -> Option<ResolvedVc<K>>
where
K: VcValueTrait + ?Sized,
{
// `RawVc::TaskCell` already contains all the type information needed to check this
// sidecast, so we don't need to read the underlying cell!
let raw_vc = this.node.node;
raw_vc
.resolved_has_trait(<K as VcValueTrait>::get_trait_type_id())
.then_some(ResolvedVc {
node: Vc {
node: raw_vc,
_t: PhantomData,
},
})
}

/// Attempts to downcast the given `ResolvedVc<Box<dyn T>>` to a `ResolvedVc<K>`, where `K`
Expand Down

0 comments on commit 4078bf9

Please sign in to comment.