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

Implement resolution for local Vcs #8826

Closed
wants to merge 1 commit into from
Closed

Conversation

bgw
Copy link
Member

@bgw bgw commented Jul 24, 2024

Description

Builds on the minimal implementation of local Vcs in #8780

At some point, we need to return local Vcs or pass them to another function as an argument. However, local Vcs are only valid within the context of the current task.

The solution is that we need to convert local Vcs to non-local Vcs, and the easiest way to do that is to .resolve() it!

This PR creates the new non-local cell on the current task, re-using the SharedReference the was used for the local cell, along with it's type id information.

RawVc lacks information about the compile-time type T. Even Vc may know the real type of T if the Vc has been downcast to Vc<Box<dyn ...>>. To solve for this, we perform a lookup of the VcCellMode::raw_cell method using the type id (I added raw_cell in #8847).

Testing Instructions

cargo nextest r -p turbo-tasks -p turbo-tasks-memory

Copy link

vercel bot commented Jul 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 29, 2024 9:05pm
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 29, 2024 9:05pm
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jul 29, 2024 9:05pm

Copy link
Member Author

bgw commented Jul 24, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bgw and the rest of your teammates on Graphite Graphite

Copy link
Contributor

github-actions bot commented Jul 24, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Jul 24, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@bgw bgw force-pushed the bgw/local-cell branch from eeeda38 to 4cabdf1 Compare July 26, 2024 19:57
@bgw bgw force-pushed the bgw/local-vc-resolve branch from b8bcdb0 to f5e96f6 Compare July 26, 2024 19:57
@bgw bgw force-pushed the bgw/local-cell branch from 4cabdf1 to 10b9aa5 Compare July 26, 2024 22:07
@bgw bgw force-pushed the bgw/local-vc-resolve branch from f5e96f6 to e797645 Compare July 26, 2024 22:20
@bgw bgw force-pushed the bgw/local-vc-resolve branch from e797645 to c077f59 Compare July 26, 2024 23:38
@bgw bgw force-pushed the bgw/local-cell branch from 10b9aa5 to 02b6fb4 Compare July 26, 2024 23:47
@bgw bgw force-pushed the bgw/local-vc-resolve branch from c077f59 to aad914c Compare July 26, 2024 23:47
@bgw bgw marked this pull request as ready for review July 26, 2024 23:47
@bgw bgw requested a review from a team as a code owner July 26, 2024 23:47
@bgw bgw force-pushed the bgw/local-cell branch from 02b6fb4 to ba7d88f Compare July 29, 2024 17:54
@bgw bgw force-pushed the bgw/local-vc-resolve branch from aad914c to 4c2a735 Compare July 29, 2024 17:54
@bgw
Copy link
Member Author

bgw commented Aug 2, 2024

Moved to the next.js repo: vercel/next.js#68472

@bgw bgw closed this Aug 2, 2024
@bgw bgw deleted the bgw/local-vc-resolve branch August 4, 2024 19:50
bgw added a commit to vercel/next.js that referenced this pull request Aug 5, 2024
*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8826

### Description

Builds on the minimal implementation of local Vcs in #68469

At some point, we need to return local Vcs or pass them to another
function as an argument. However, local Vcs are only valid within the
context of the current task.

The solution is that we need to convert local `Vc`s to non-local `Vc`s,
and the easiest way to do that is to `.resolve()` it!

This PR creates the new non-local cell on the current task, re-using the
SharedReference the was used for the local cell, along with it's type id
information.

`RawVc` lacks information about the compile-time type `T`. Even `Vc` may
know the real type of `T` if the `Vc` has been downcast to `Vc<Box<dyn
...>>`. To solve for this, we perform a lookup of the
`VcCellMode::raw_cell` method using the type id (I added `raw_cell` in
#68467).

### Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 14, 2024
*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8826

### Description

Builds on the minimal implementation of local Vcs in #68469

At some point, we need to return local Vcs or pass them to another
function as an argument. However, local Vcs are only valid within the
context of the current task.

The solution is that we need to convert local `Vc`s to non-local `Vc`s,
and the easiest way to do that is to `.resolve()` it!

This PR creates the new non-local cell on the current task, re-using the
SharedReference the was used for the local cell, along with it's type id
information.

`RawVc` lacks information about the compile-time type `T`. Even `Vc` may
know the real type of `T` if the `Vc` has been downcast to `Vc<Box<dyn
...>>`. To solve for this, we perform a lookup of the
`VcCellMode::raw_cell` method using the type id (I added `raw_cell` in
#68467).

### Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 15, 2024
*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8826

### Description

Builds on the minimal implementation of local Vcs in #68469

At some point, we need to return local Vcs or pass them to another
function as an argument. However, local Vcs are only valid within the
context of the current task.

The solution is that we need to convert local `Vc`s to non-local `Vc`s,
and the easiest way to do that is to `.resolve()` it!

This PR creates the new non-local cell on the current task, re-using the
SharedReference the was used for the local cell, along with it's type id
information.

`RawVc` lacks information about the compile-time type `T`. Even `Vc` may
know the real type of `T` if the `Vc` has been downcast to `Vc<Box<dyn
...>>`. To solve for this, we perform a lookup of the
`VcCellMode::raw_cell` method using the type id (I added `raw_cell` in
#68467).

### Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 16, 2024
*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8826

### Description

Builds on the minimal implementation of local Vcs in #68469

At some point, we need to return local Vcs or pass them to another
function as an argument. However, local Vcs are only valid within the
context of the current task.

The solution is that we need to convert local `Vc`s to non-local `Vc`s,
and the easiest way to do that is to `.resolve()` it!

This PR creates the new non-local cell on the current task, re-using the
SharedReference the was used for the local cell, along with it's type id
information.

`RawVc` lacks information about the compile-time type `T`. Even `Vc` may
know the real type of `T` if the `Vc` has been downcast to `Vc<Box<dyn
...>>`. To solve for this, we perform a lookup of the
`VcCellMode::raw_cell` method using the type id (I added `raw_cell` in
#68467).

### Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants