feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks #69126

bgw · 2024-08-21T05:10:22Z

When calling a turbo tasks function with unresolved arguments, we create a wrapper CachedTaskType::ResolveNative or ResolveTrait task.

These are quite frequently created, and their behavior is constrained and well-defined, so we should turn them into local tasks that aren't cached in the backend.

This logic is gated behind --features turbo-tasks/local_resolution, as it has some known issues.

Known Issues

A RawVc::LocalOutput can currently only be resolved within it's parent task.

The long-term plan is for everything to use ResolvedVc, so that this assumption is safe.

The short-term plan is to store local outputs more globally. Design here: https://www.notion.so/vercel/RawVc-LocalOutput-aede5f463f594ca58396eb3fdaffd865?pvs=4#02af94e3b9b64a3690f3e467abc0a47e

Test Plan

cargo nextest r --features turbo-tasks/local_resolution -p turbo-tasks -p turbo-tasks-memory

bgw · 2024-08-21T05:10:42Z

feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks #69126 👈 (View in Graphite)
refactor(turbo-tasks-memory): Minor cleanup of code in turbo-tasks-memory/src/task.rs #69505
refactor(turbo-tasks): Move OutputContent from memory backend into turbo-tasks, represent Empty as None #69473
refactor(turbo-tasks): Remove some format_description logic from memory backend #69472
refactor(turbo-tasks): Add stubs for RawVc::TaskOutput #68908
chore(turbo-tasks): Move Invalidator struct from manager.rs to invalidation.rs #69073
refactor(turbo-tasks) Add a higher-level task-local state API for the Backend trait #68996
refactor(turbo-tasks): Split local and global task state #68963
refactor(turbo-tasks): Make CURRENT_TASK_STATE shareable across multiple tokio tasks / threads, wait for detached work to finish before exiting a turbo task #68921
refactor(turbo-tasks): Use an enum for strongly_consistent/ReadConsistency instead of a bool #68913
perf(turbo-tasks-macros): Move assert_returns_resolved_value into helper module #68867
refactor(turbo-tasks): Add TaskPersistence enum for task creation functions #68866
[Turbopack] Transient Tasks #68436 : 1 other dependent PR (#68437 )
[Turbopack] run tests multiple times to test caching #68722
[Turbopack] serialization fixups #68715
canary

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

ijjk · 2024-08-21T05:24:05Z

Stats from current PR

Default Build (Increase detected ⚠️)

General

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
buildDuration	21.4s	19.6s	N/A
buildDurationCached	18.7s	15.7s	N/A
nodeModulesSize	410 MB	410 MB	N/A
nextStartRea..uration (ms)	547ms	537ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
1187-HASH.js gzip	51 kB	51 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.36 kB	5.36 kB	N/A
bccd1874-HASH.js gzip	53 kB	53 kB	N/A
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	233 B	235 B	N/A
main-HASH.js gzip	34.1 kB	34 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	0 B	0 B	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.49 kB	4.49 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.34 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
_buildManifest.js gzip	749 B	746 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
index.html gzip	524 B	523 B	N/A
link.html gzip	539 B	537 B	N/A
withRouter.html gzip	520 B	519 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
edge-ssr.js gzip	128 kB	128 kB	N/A
page.js gzip	204 kB	204 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
middleware-b..fest.js gzip	670 B	668 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.2 kB	31.2 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
523-experime...dev.js gzip	322 B	322 B	✓
523.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	323 kB	323 kB	✓
app-page-exp..prod.js gzip	127 kB	127 kB	✓
app-page-tur..prod.js gzip	140 kB	140 kB	✓
app-page-tur..prod.js gzip	135 kB	135 kB	✓
app-page.run...dev.js gzip	314 kB	314 kB	✓
app-page.run..prod.js gzip	123 kB	123 kB	✓
app-route-ex...dev.js gzip	37.4 kB	37.4 kB	✓
app-route-ex..prod.js gzip	25.5 kB	25.5 kB	✓
app-route-tu..prod.js gzip	25.5 kB	25.5 kB	✓
app-route-tu..prod.js gzip	25.3 kB	25.3 kB	✓
app-route.ru...dev.js gzip	39 kB	39 kB	✓
app-route.ru..prod.js gzip	25.3 kB	25.3 kB	✓
pages-api-tu..prod.js gzip	9.69 kB	9.69 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.68 kB	9.68 kB	✓
pages-turbo...prod.js gzip	21.7 kB	21.7 kB	✓
pages.runtim...dev.js gzip	27.4 kB	27.4 kB	✓
pages.runtim..prod.js gzip	21.7 kB	21.7 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	✓
Overall change	2.36 MB	2.36 MB	✓

build cache Overall increase ⚠️

	vercel/next.js canary	vercel/next.js bgw/schedule-local-task	Change
0.pack gzip	2.06 MB	2.06 MB	⚠️ +642 B
index.pack gzip	72.3 kB	73.3 kB	⚠️ +967 B
Overall change	2.14 MB	2.14 MB	⚠️ +1.61 kB

Diff details

Diff for main-HASH.js

Diff too large to display

Commit: bddbd9e

ijjk · 2024-08-21T21:35:19Z

Tests Passed

sokra · 2024-09-04T07:02:53Z

turbopack/crates/turbo-tasks/src/manager.rs

+            let local_task_id = gts_write.create_local_task(task_type, persistence);
+            RawVc::LocalOutput(gts_write.task_id, local_task_id)
+        });
+        #[cfg(not(feature = "local_resolution"))]
        match persistence {
            TaskPersistence::LocalCells => {


Looks like TaskPersistence::LocalCells is not needed for local tasks at all. Maybe we could remove this variant (resp. modify the previous PR to never introduce it).

I like that local tasks are completely handled by the manager and not leaking into the backend.

For the current implementation where local tasks are only used for resolution it's not needed. We don't have arbitrary local tasks, so no task function can meaningfully take or return local cells.

However, I think local cells will be useful eventually. If we're calling a local task, we should allow local cells to be passed in as arguments or returned as outputs without requiring that they be resolved to a global cell. That will reduce the overall number of global cells that we create and store.

Once everything is a ResolvedValue, we can experiment with changing the behavior of Vc::cell to always create a local cell, deferring the construction of a real cell until the Vc is resolved.

## Why? https://www.notion.so/vercel/RawVc-LocalOutput-aede5f463f594ca58396eb3fdaffd865 When returning from a function using `local_cells`, we need to return a `RawVc`, but we don't know that the output type is yet, so we need a `RawVc` variant that does not require a type. This is fundamentally the same problem that `RawVc::TaskOutput` solves. `RawVc::LocalOutput` is the same thing, but for functions using `local_cells`. ## Implementation There's an (partially broken) implementation in #69126. This PR exists to help break #69126 into more reviewable chunks.

…rbo-tasks, represent Empty as None (#69473) This is preparation for local tasks/cells in #69126. - Moves this into the `turbo-tasks` crate so that it can be used with local cells/outputs in #69126. - Remove the `Empty` state from the enum because the implementation in #69126 does not currently need that. This can still be represented using `None`.

…ks as local tasks

@sokra

…ce a macro annotation for creating OperationVc types (#72871) Introduces a new `operation` argument to the `#[turbo_tasks::function]` macro for creating functions that return `OperationVc`: ``` // this function's external signature returns an `OperationVc` #[turbo_tasks::function(operation)] async fn multiply(value: OperationVc<i32>, coefficient: i32) -> Result<Vc<i32>> { Ok(Vc::cell(*value.connect().await? * coefficient)) } ``` This is important to solve a few major footguns with manually-constructed `OperationVc`s: - It can be hard to know if a `Vc` is an operation or not, and the only warning you'd get if you got it wrong was a runtime error. - Local-task-based resolution (#69126) will implicitly create local `Vc`s if an argument isn't resolved. - ~We want to enforce that all arguments to `OperationVc`-returning functions are also `OperationVc`s or non-`Vc` values. Otherwise you could end up with a stale/wrong `OperationVc`.~ Scrapped, this was too hard/impractical, see below. This removes the public constructor. Methods are not currently supported because: 1. Operations are uncommon, and it's not worth the effort, IMO. 2. There's no way to make it work for dynamic-dispatched method calls, as we cannot resolve the type of `self`. It could be made to work for inherent impls and non-object trait types. --- This is basically implementing @sokra's TODO comment in the old `OperationVc::new` constructor: ``` // TODO to avoid this runtime check, we should mark functions with `(operation)` and return // a OperationVc directly ``` But with assertions for the function arguments, based on some discussion in DMs: <img src="https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/HAZVitxRNnZz8QMiPn4a/13952614-f421-4c8a-99b8-3ce5f19715ae.png" width="600"/> We allow *either* `ResolvedVc` or `OperationVc` as arguments because: - Only accepting `OperationVc` arguments was impossible to refactor to, as this made it "viral" and there are too many places where we need to use operations that have too many dependencies. - While it makes sense for `State` to require `OperationVc` as arguments to any operation (keeps the whole dependency/call tree alive), for collectibles, sometimes you want the arguments to be `ResolvedVc`. It's just a matter of if you want to include collectibles generated when creating the arguments or not.

ijjk added the created-by: Turbopack team PRs by the Turbopack team. label Aug 21, 2024

bgw changed the title ~~Add schedule_local_task method~~ draft(turbo-tasks): Add schedule_local_task method Aug 21, 2024

bgw force-pushed the bgw/stub-task-output branch from 21b123a to 06c1a4f Compare August 22, 2024 00:35

bgw force-pushed the bgw/schedule-local-task branch from bf4e236 to 5f7cee9 Compare August 22, 2024 00:35

bgw changed the title ~~draft(turbo-tasks): Add schedule_local_task method~~ feat(turbo-tasks): Optionally schedule ResolveNative Aug 22, 2024

bgw changed the title ~~feat(turbo-tasks): Optionally schedule ResolveNative~~ feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks Aug 22, 2024

bgw force-pushed the bgw/stub-task-output branch from 06c1a4f to a718d9e Compare August 29, 2024 19:50

bgw force-pushed the bgw/schedule-local-task branch from 5f7cee9 to 8237476 Compare August 29, 2024 19:50

bgw force-pushed the bgw/stub-task-output branch from a718d9e to bcd07cf Compare August 29, 2024 22:32

bgw force-pushed the bgw/schedule-local-task branch from 8237476 to 694aa77 Compare August 29, 2024 22:32

bgw changed the base branch from bgw/stub-task-output to bgw/move-output-content August 29, 2024 22:32

This was referenced Aug 29, 2024

refactor(turbo-tasks): Remove some format_description logic from memory backend #69472

Merged

refactor(turbo-tasks): Move OutputContent from memory backend into turbo-tasks, represent Empty as None #69473

Merged

bgw force-pushed the bgw/move-output-content branch from a00965d to 7ce8ac4 Compare August 30, 2024 18:00

bgw force-pushed the bgw/schedule-local-task branch from 694aa77 to 07ce967 Compare August 30, 2024 18:00

bgw changed the base branch from bgw/move-output-content to bgw/memory-task-cleanup August 30, 2024 18:00

bgw mentioned this pull request Aug 30, 2024

refactor(turbo-tasks-memory): Minor cleanup of code in turbo-tasks-memory/src/task.rs #69505

Merged

bgw force-pushed the bgw/memory-task-cleanup branch from 9e272d6 to 52ea711 Compare September 4, 2024 00:25

bgw force-pushed the bgw/schedule-local-task branch from 07ce967 to f292e7b Compare September 4, 2024 00:26

bgw force-pushed the bgw/memory-task-cleanup branch from 52ea711 to 883ebe2 Compare September 4, 2024 01:45

bgw force-pushed the bgw/schedule-local-task branch from f292e7b to 78ff091 Compare September 4, 2024 01:48

sokra reviewed Sep 4, 2024

View reviewed changes

bgw force-pushed the bgw/memory-task-cleanup branch from 883ebe2 to 155e5e1 Compare September 4, 2024 17:06

bgw force-pushed the bgw/schedule-local-task branch from 78ff091 to 36a864d Compare September 4, 2024 17:07

bgw force-pushed the bgw/memory-task-cleanup branch from 155e5e1 to 07ef1b0 Compare September 4, 2024 17:37

bgw force-pushed the bgw/schedule-local-task branch from 36a864d to 7cef0ba Compare September 4, 2024 17:41

bgw force-pushed the bgw/memory-task-cleanup branch from 07ef1b0 to e820d88 Compare September 4, 2024 18:37

bgw force-pushed the bgw/schedule-local-task branch from 7cef0ba to 8a6276b Compare September 4, 2024 18:40

bgw force-pushed the bgw/memory-task-cleanup branch from e820d88 to 2dc6d23 Compare September 4, 2024 19:05

bgw force-pushed the bgw/schedule-local-task branch from 8a6276b to 75ca016 Compare September 4, 2024 19:05

bgw force-pushed the bgw/memory-task-cleanup branch from 2dc6d23 to a71b79c Compare September 10, 2024 18:01

bgw force-pushed the bgw/schedule-local-task branch from 75ca016 to 4439d28 Compare September 10, 2024 18:01

bgw force-pushed the bgw/memory-task-cleanup branch from a71b79c to 9025802 Compare September 10, 2024 18:22

bgw force-pushed the bgw/schedule-local-task branch from 4439d28 to d886648 Compare September 10, 2024 18:22

bgw force-pushed the bgw/memory-task-cleanup branch from 9025802 to 4d838e7 Compare September 10, 2024 18:41

bgw force-pushed the bgw/schedule-local-task branch from d886648 to f4aa228 Compare September 10, 2024 18:44

bgw force-pushed the bgw/memory-task-cleanup branch from 4d838e7 to 08b3660 Compare September 10, 2024 18:57

bgw force-pushed the bgw/schedule-local-task branch 2 times, most recently from 4a018d2 to 8189bd1 Compare September 10, 2024 19:31

bgw changed the base branch from bgw/memory-task-cleanup to graphite-base/69126 September 10, 2024 19:39

bgw force-pushed the graphite-base/69126 branch from 08b3660 to cfdd5dc Compare September 10, 2024 19:42

bgw force-pushed the bgw/schedule-local-task branch from 8189bd1 to ab5ac04 Compare September 10, 2024 19:42

bgw changed the base branch from graphite-base/69126 to canary September 10, 2024 19:42

bgw mentioned this pull request Nov 16, 2024

refactor(turbo-tasks): Remove public OperationVc constructor, introduce a macro annotation for creating OperationVc types #72871

Merged

feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tas…

bddbd9e

…ks as local tasks

bgw force-pushed the bgw/schedule-local-task branch from ab5ac04 to bddbd9e Compare December 17, 2024 00:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks #69126

feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks #69126

bgw commented Aug 21, 2024 •

edited

Loading

bgw commented Aug 21, 2024 •

edited

Loading

ijjk commented Aug 21, 2024 •

edited

Loading

ijjk commented Aug 21, 2024 •

edited

Loading

sokra Sep 4, 2024

bgw Sep 10, 2024

feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks #69126

Are you sure you want to change the base?

feat(turbo-tasks): Optionally schedule ResolveNative/ResolveTrait tasks as local tasks #69126

Conversation

bgw commented Aug 21, 2024 • edited Loading

Known Issues

Test Plan

bgw commented Aug 21, 2024 • edited Loading

ijjk commented Aug 21, 2024 • edited Loading

Stats from current PR

ijjk commented Aug 21, 2024 • edited Loading

Tests Passed

sokra Sep 4, 2024

Choose a reason for hiding this comment

bgw Sep 10, 2024

Choose a reason for hiding this comment

bgw commented Aug 21, 2024 •

edited

Loading

bgw commented Aug 21, 2024 •

edited

Loading

ijjk commented Aug 21, 2024 •

edited

Loading

ijjk commented Aug 21, 2024 •

edited

Loading