Skip to content

Commit 099b1e3

Browse files
authored
[turbopack] Adopt inventory for our registry (#83074)
## What? Leverage the `inventory` crate to automatically register turbo tasks objects using specialized linker sections. See https://docs.rs/inventory/0.3.21/inventory/ for details For NativeFunction and TraitType objects this was trivial and simplified the registry, for example. * functions only need the global registry during PC _deserialization_ so many builds can skip the cost. * Even when we do allocate it, we can use a normal `HashMap` instead of a `DashMap` which should be a minor performance benefit on its own (no locks, smaller in ram) * TraitType objects achieve similar benefits. The tricky part of course was trait impls: * Every `ValueType` needs to know all the trait methods it implements * To support calling `turbo_tasks::functions` on `Vc<Box<dyn T>>` * every `TraitType` needs to know all the structs that implement it * To support dereferencing a trait ref, aka `TraitRef<Box<dyn T>>` -> `&dyn T` to support calling non-turbotasks functions This required two more registries which more or less followed the existing pattern. One benefit of the new `inventory` approach is that the `VTableRegistry` drops its interior mutability (and associated locks). Constructing VTableRegistry objects also becomes lazy which is useful since many are not needed. So overall this should be equivalent but slightly better and it allows us to drop the build scripts (See #83156 ) One interesting note is determinism of `TraitTypeId` and `ValueTypeId`. Previously the values assigned were determined by the order of registrations in the build script. With inventory they are registered in an undefined order (im guessing it is deterministic based on linker input order, but inventory makes no guarantees). As far as i can tell, this does not matter, but in the interest of determinism i have sorted the values based on their names. In the future we can use this determinism to optimize persistence ## Why? By colocating registration with the proc macros we can simplify the registration logic. Switching to 'pull' based registration also simplifies a number of datastructures and allows us to drop some RWLocks ## Interesting consequences All the global type names are changing, dropping the `@TODO::` This `TODO` was a placeholder for a content hash of the current crate. This is to support a hypothetical idea where at least some of the persistent cache could survive turbopack updates. However based on team discussions we don't currently see a clear solution here. If we want to do this in the future i think we could still leverage hashes in names but we would need to compose them in a different way (e.g. `build.rs` script sets an `env` var that the proc macro reads with the `env!` macro)
1 parent 10e70b6 commit 099b1e3

26 files changed

+470
-797
lines changed

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ urlencoding = "2.1.2"
444444
vergen = { version = "9.0.6", features = ["cargo"] }
445445
vergen-gitcl = { version = "1.0.8", features = ["cargo"] }
446446
webbrowser = "0.8.7"
447+
inventory = "0.3.21"
447448

448449
[patch.crates-io]
449450
hyper = { git = "https://github.com/bgw/hyper-rs.git", branch = "v1.6.0-with-macos-intel-miscompilation-workaround" }

turbopack/crates/turbo-tasks-backend/tests/task_statistics.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,11 @@ fn enable_stats() {
254254

255255
fn stats_json() -> serde_json::Value {
256256
let tt = turbo_tasks::turbo_tasks();
257-
remove_crate_and_hashes(serde_json::to_value(tt.task_statistics().get()).unwrap())
257+
remove_crate(serde_json::to_value(tt.task_statistics().get()).unwrap())
258258
}
259259

260-
// Global task identifiers can contain a hash of the crate and dependencies.
261-
// Remove that so that we can compare against a stable value in tests.
262-
fn remove_crate_and_hashes(mut json: serde_json::Value) -> serde_json::Value {
260+
// Global task identifiers can contain the crate name, remove it to simplify test assertions
261+
fn remove_crate(mut json: serde_json::Value) -> serde_json::Value {
263262
static HASH_RE: Lazy<Regex> = Lazy::new(|| Regex::new("^[^:@]+@[^:]+:+").unwrap());
264263
match &mut json {
265264
serde_json::Value::Object(map) => {

turbopack/crates/turbo-tasks-backend/tests/trace_transient.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ use turbo_tasks_testing::{Registration, register, run_without_cache_check};
99
static REGISTRATION: Registration = register!();
1010

1111
const EXPECTED_TRACE: &str = "\
12-
Adder::add_method (read cell of type turbo-tasks@TODO::::primitives::u64)
12+
Adder::add_method (read cell of type turbo-tasks@turbo_tasks::primitives::u64)
1313
self:
14-
Adder::new (read cell of type turbo-tasks-backend@TODO::::Adder)
14+
Adder::new (read cell of type turbo-tasks-backend@trace_transient::Adder)
1515
args:
16-
unknown transient task (read cell of type turbo-tasks@TODO::::primitives::unit)
16+
unknown transient task (read cell of type turbo-tasks@turbo_tasks::primitives::())
1717
args:
18-
unknown transient task (read cell of type turbo-tasks@TODO::::primitives::u16)
19-
unknown transient task (read cell of type turbo-tasks@TODO::::primitives::u32)";
18+
unknown transient task (read cell of type turbo-tasks@turbo_tasks::primitives::u16)
19+
unknown transient task (read cell of type turbo-tasks@turbo_tasks::primitives::u32)";
2020

2121
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2222
async fn test_trace_transient() {

0 commit comments

Comments
 (0)