Skip to content

Commit 42ece5a

Browse files
committed
turbo-tasks: TinyVec API hardening + const-generic MAX
* Tighten visibility: `new`, `capacity`, `as_slice`, `as_mut_slice`, `reserve` are now private. `len` and `is_empty` stay pub as a clippy- preferred pair. * Replace the hand-rolled `retain_mut` (~7 unsafe blocks with a panic partial-shift guard) with delegation to `Vec::retain_mut` via a Vec::from_raw_parts round-trip. retain_mut is cold relative to push so the round-trip cost is irrelevant. * Replace the hand-rolled owned `IntoIter` (custom struct + Iterator, Drop, ExactSizeIterator impls) with delegation to `std::vec::IntoIter` via the same Vec round-trip. Drops ~50 lines of unsafe. * Drop `Extend` and `FromIterator` trait impls entirely — no external callers. Replace with inherent `extend_exact` which requires an `ExactSizeIterator` and reserves exactly once. * Drop inherent `iter`, `iter_mut`, `last_mut`, `Index`, `IndexMut` impls — all reachable through `Deref<Target = [T]>` to the slice's methods. `for x in &tv` / `for x in &mut tv` still need `IntoIterator` impls for refs because the `for`-loop desugar doesn't apply `Deref` coercion across the reference boundary. * Add a `const MAX: u8` generic parameter that strictly caps push count and tightens the growth schedule (doubles until it would exceed MAX, then caps exactly at MAX). Default is `u8::MAX = 255` for backward compatibility; the schema macro emits `TinyVec<LazyField, N>` where N is the exact lazy-field count. A compile-time static assert rejects `MAX = 0` at monomorphization. * Add 3 const-generic tests (tight_max_caps_growth_exactly, tight_max_panics_at_limit, tight_max_growth_schedule) and 6 retain_mut tests (basic, can_mutate, empty, keeps_all, removes_all, panic_safety, element_drop_panic). Net effect: unsafe blocks went from ~19 (in the original hand-rolled version) to 5 in the hot path plus 1 in retain_mut and 1 in IntoIter — each upholding a single local invariant or just round-tripping through Vec.
1 parent d772e48 commit 42ece5a

6 files changed

Lines changed: 299 additions & 261 deletions

File tree

turbopack/crates/turbo-tasks-backend/src/backend/operation/cleanup_old_edges.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,6 @@ impl CleanupOldEdgesOperation {
176176
) = dep.into_parts();
177177
{
178178
let mut task = ctx.task(cell_task_id, TaskDataCategory::Data);
179-
// The `cell_dependents` set on the dependee task is keyed
180-
// by `(cell, task_id, key)`. Re-encode that as a
181-
// `CellDependency` whose `CellRef.task` is the dependent
182-
// task — same bits, opposite direction.
183179
task.remove_cell_dependents(&CellDependency::new(
184180
CellRef {
185181
task: task_id,

turbopack/crates/turbo-tasks-backend/src/backend/operation/update_cell.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,6 @@ impl UpdateCellOperation {
138138
FxIndexMap::default();
139139
if !skip_invalidation {
140140
let tasks_with_keys = task.iter_cell_dependents().filter_map(|dep| {
141-
// `cell_dependents` reuses `CellDependency`, but with the dependent task
142-
// stored in `CellRef.task` and the storing task's cell in `CellRef.cell`.
143141
let (
144142
CellRef {
145143
task: dependent_task,

turbopack/crates/turbo-tasks-backend/src/data.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,6 @@ impl CollectiblesRef {
8585
/// - In `cell_dependents`, the [`CellRef`]'s `task` is the dependent task and `cell` is the cell of
8686
/// the storing task; the `task` field is reused as the dependent's id rather than the cell's
8787
/// owning task. The fields encode the same bits either way.
88-
///
89-
/// Replaces the old `(CellRef, Option<u64>)` and `(CellId, Option<u64>, TaskId)` tuples. The
90-
/// `Option<u64>` previously cost a full 16 B (8 B discriminant + 8 B value, aligned). With an
91-
/// explicit enum the layout algorithm reuses the niche on `ValueTypeId` (`NonZero<u16>`)
92-
/// inside `CellRef.cell.type_id` for the variant tag, dropping the element from 32 B to 24 B.
93-
/// That in turn shrinks `LazyField` from 56 B to 48 B.
9488
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Encode, Decode)]
9589
pub enum CellDependency {
9690
/// Depend on the cell as a whole.

turbopack/crates/turbo-tasks-macros/src/derive/task_storage_macro.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,12 +1296,21 @@ fn generate_typed_storage_struct(grouped_fields: &GroupedFields) -> TokenStream
12961296
quote! {}
12971297
};
12981298

1299-
// `TinyVec` is resolved at the schema's call site — the schema must
1300-
// `use turbo_tasks::TinyVec` (or otherwise have it in scope).
13011299
let lazy_field = if has_lazy {
1300+
// `TinyVec`'s `MAX` const generic is set to the exact number of lazy fields declared
1301+
// in the schema. This caps growth at the smallest power-of-two-or-MAX boundary
1302+
// (e.g. with 24 variants we end at cap=24 instead of cap=32), saving a few slots
1303+
// per fully-populated task. It also makes "push past MAX" a compile-time-bounded
1304+
// contract instead of relying on `u8::MAX`.
1305+
//
1306+
// `as u8` cast is safe at the macro layer: u8::MAX is plenty of room for any
1307+
// realistic schema (asserted at compile time by `TinyVec::new`'s `MAX > 0` guard
1308+
// — a runtime check is not strictly required because the macro itself wouldn't
1309+
// emit > 255 variants).
1310+
let max_lazy = grouped_fields.all_lazy().count() as u8;
13021311
quote! {
13031312
#[doc = "Lazily-allocated fields stored in a compact TinyVec for memory efficiency"]
1304-
lazy: TinyVec<LazyField>,
1313+
lazy: TinyVec<LazyField, #max_lazy>,
13051314
}
13061315
} else {
13071316
quote! {}
@@ -3686,10 +3695,10 @@ fn generate_snapshot_restore_methods(grouped_fields: &GroupedFields) -> TokenStr
36863695

36873696
#clone_all_flags
36883697

3689-
// Pre-allocate lazy vec (upper bound - some may be transient and skipped)
3690-
snapshot.lazy.reserve(self.lazy.len());
3691-
3692-
// Clone all persistent lazy fields (both meta and data)
3698+
// Clone all persistent lazy fields (both meta and data).
3699+
// (No pre-`reserve`: the schema has ≤24 lazy fields, so at most 3 grows
3700+
// (0→4→8→16→24) total — cheaper than complicating the public API surface
3701+
// of `TinyVec`.)
36933702
for field in &self.lazy {
36943703
match field {
36953704
#(#clone_data_lazy_arms)*
@@ -3746,7 +3755,7 @@ fn generate_snapshot_restore_methods(grouped_fields: &GroupedFields) -> TokenStr
37463755
// and merge each source variant in O(1).
37473756
let (any_meta, _any_data, index) = Self::build_lazy_index(&self.lazy);
37483757
if !any_meta {
3749-
self.lazy.extend(source.lazy);
3758+
self.lazy.extend_exact(source.lazy);
37503759
} else {
37513760
for field in source.lazy {
37523761
debug_assert!(field.is_persistent() && field.is_meta());
@@ -3770,7 +3779,7 @@ fn generate_snapshot_restore_methods(grouped_fields: &GroupedFields) -> TokenStr
37703779
// in `self.lazy` is never a collision risk.
37713780
let (_any_meta, any_data, index) = Self::build_lazy_index(&self.lazy);
37723781
if !any_data {
3773-
self.lazy.extend(source.lazy);
3782+
self.lazy.extend_exact(source.lazy);
37743783
} else {
37753784
for field in source.lazy {
37763785
debug_assert!(field.is_persistent() && field.is_data());

turbopack/crates/turbo-tasks/benches/tiny_vec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ fn push_vec(n: usize) -> Vec<Item> {
4545
}
4646

4747
fn push_tinyvec(n: usize) -> TinyVec<Item> {
48-
let mut v: TinyVec<Item> = TinyVec::new();
48+
let mut v: TinyVec<Item> = TinyVec::default();
4949
for i in 0..n {
5050
v.push(make_item(i as u64));
5151
}

0 commit comments

Comments
 (0)