Skip to content

Commit d772e48

Browse files
committed
turbo-tasks: task-storage memory wins
* Migrate Arc<CachedTaskType> to triomphe::Arc via CachedTaskTypeArc newtype. Saves one usize per allocation (no weak count) and avoids the weak-count CAS in drop_slow. * Niche-encode cell_dependencies / cell_dependents elements as a CellDependency enum that reuses the niche on ValueTypeId (NonZero<u16>) for the variant tag. Element size drops from 32 B to 24 B; LazyField from 56 B to 48 B. * Replace TaskStorage::lazy: Vec<LazyField> with a custom 16 B TinyVec (u8 len + u8 cap, the schema's max field count is well under 255). Drops size_of::<TaskStorage>() from 136 B to 128 B. The included micro-benchmarks show TinyVec push is 11-32% faster than Vec across realistic sizes and iter is neutral. * Add CellDependency::into_parts() and use it in iter_cell_dependents / iter_cell_dependencies hot loops to avoid checking the enum discriminant twice via back-to-back cell_ref() + key() calls. Cumulative MaxRSS reduction is ~2.9% on a representative Next.js build (measured on a real project, 15 runs each side, t=-13.7 vs canary).
1 parent ec0279c commit d772e48

13 files changed

Lines changed: 990 additions & 90 deletions

File tree

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ use turbo_tasks::{
3434
TaskId, TaskPersistence, TaskPriority, TraitTypeId, TurboTasksBackendApi, TurboTasksPanic,
3535
ValueTypeId,
3636
backend::{
37-
Backend, CachedTaskType, CellContent, CellHash, TaskExecutionSpec, TransientTaskType,
38-
TurboTaskContextError, TurboTaskLocalContextError, TurboTasksError,
37+
Backend, CachedTaskType, CachedTaskTypeArc, CellContent, CellHash, TaskExecutionSpec,
38+
TransientTaskType, TurboTaskContextError, TurboTaskLocalContextError, TurboTasksError,
3939
TurboTasksExecutionError, TurboTasksExecutionErrorMessage, TypedCellContent,
4040
VerificationMode,
4141
},
@@ -70,8 +70,8 @@ use crate::{
7070
},
7171
backing_storage::{BackingStorage, SnapshotItem, compute_task_type_hash},
7272
data::{
73-
ActivenessState, CellRef, CollectibleRef, CollectiblesRef, Dirtyness, InProgressCellState,
74-
InProgressState, InProgressStateInner, OutputValue, TransientTask,
73+
ActivenessState, CellDependency, CellRef, CollectibleRef, CollectiblesRef, Dirtyness,
74+
InProgressCellState, InProgressState, InProgressStateInner, OutputValue, TransientTask,
7575
},
7676
error::TaskError,
7777
utils::{
@@ -785,7 +785,8 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
785785
&& (!task.immutable() || cfg!(feature = "verify_immutable"))
786786
{
787787
let reader = reader.unwrap();
788-
let _ = task.add_cell_dependents((cell, key, reader));
788+
let _ = task
789+
.add_cell_dependents(CellDependency::new(CellRef { task: reader, cell }, key));
789790
drop(task);
790791

791792
// Note: We use `task_pair` earlier to lock the task and its reader at the same
@@ -797,8 +798,9 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
797798
task: task_id,
798799
cell,
799800
};
800-
if !reader_task.remove_outdated_cell_dependencies(&(target, key)) {
801-
let _ = reader_task.add_cell_dependencies((target, key));
801+
let dep = CellDependency::new(target, key);
802+
if !reader_task.remove_outdated_cell_dependencies(&dep) {
803+
let _ = reader_task.add_cell_dependencies(dep);
802804
}
803805
drop(reader_task);
804806
}
@@ -1526,7 +1528,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
15261528
// Only now do we force the allocation.
15271529
// NOTE: if our caller had to perform resolution, then this will have already
15281530
// been boxed and take_box just takes it.
1529-
let task_type = Arc::new(CachedTaskType {
1531+
let task_type = CachedTaskTypeArc::new(CachedTaskType {
15301532
native_fn,
15311533
this,
15321534
arg: arg.take_box(),
@@ -1757,7 +1759,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
17571759
}
17581760
}
17591761

1760-
fn debug_get_cached_task_type(&self, task_id: TaskId) -> Option<Arc<CachedTaskType>> {
1762+
fn debug_get_cached_task_type(&self, task_id: TaskId) -> Option<CachedTaskTypeArc> {
17611763
let task = self.storage.access_mut(task_id);
17621764
task.get_persistent_task_type().cloned()
17631765
}
@@ -2197,7 +2199,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
21972199
Some(
21982200
// Collect all dependencies on tasks to check if all dependencies are immutable
21992201
task.iter_output_dependencies()
2200-
.chain(task.iter_cell_dependencies().map(|(target, _key)| target.task))
2202+
.chain(task.iter_cell_dependencies().map(|dep| dep.cell_ref().task))
22012203
.collect::<FxHashSet<_>>(),
22022204
)
22032205
} else {
@@ -2236,7 +2238,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
22362238
// breaking dependency tracking.
22372239
old_edges.extend(
22382240
task.iter_outdated_cell_dependencies()
2239-
.map(|(target, key)| OutdatedEdge::CellDependency(target, key)),
2241+
.map(OutdatedEdge::CellDependency),
22402242
);
22412243
old_edges.extend(
22422244
task.iter_outdated_output_dependencies()

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::{
1717
},
1818
storage_schema::TaskStorageAccessors,
1919
},
20-
data::{CellRef, CollectibleRef, CollectiblesRef},
20+
data::{CellDependency, CellRef, CollectibleRef, CollectiblesRef},
2121
};
2222

2323
#[derive(Encode, Decode, Clone)]
@@ -48,7 +48,7 @@ impl Default for CleanupOldEdgesOperation {
4848
pub enum OutdatedEdge {
4949
Child(TaskId),
5050
Collectible(CollectibleRef, i32),
51-
CellDependency(CellRef, Option<u64>),
51+
CellDependency(CellDependency),
5252
OutputDependency(TaskId),
5353
CollectiblesDependency(CollectiblesRef),
5454
}
@@ -166,27 +166,32 @@ impl CleanupOldEdgesOperation {
166166
AggregatedDataUpdate::new().collectibles_update(collectibles),
167167
));
168168
}
169-
OutdatedEdge::CellDependency(
170-
CellRef {
171-
task: cell_task_id,
172-
cell,
173-
},
174-
key,
175-
) => {
169+
OutdatedEdge::CellDependency(dep) => {
170+
let (
171+
CellRef {
172+
task: cell_task_id,
173+
cell,
174+
},
175+
key,
176+
) = dep.into_parts();
176177
{
177178
let mut task = ctx.task(cell_task_id, TaskDataCategory::Data);
178-
task.remove_cell_dependents(&(cell, key, task_id));
179-
}
180-
{
181-
let mut task = ctx.task(task_id, TaskDataCategory::Data);
182-
task.remove_cell_dependencies(&(
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.
183+
task.remove_cell_dependents(&CellDependency::new(
183184
CellRef {
184-
task: cell_task_id,
185+
task: task_id,
185186
cell,
186187
},
187188
key,
188189
));
189190
}
191+
{
192+
let mut task = ctx.task(task_id, TaskDataCategory::Data);
193+
task.remove_cell_dependencies(&dep);
194+
}
190195
}
191196
OutdatedEdge::OutputDependency(output_task_id) => {
192197
#[cfg(feature = "trace_task_output_dependencies")]

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use tracing::info_span;
1919
use tracing::trace_span;
2020
use turbo_tasks::{
2121
CellId, DynTaskInputs, FxIndexMap, RawVc, SharedReference, TaskExecutionReason, TaskId,
22-
TaskPriority, TurboTasksBackendApi, TurboTasksCallApi, backend::CachedTaskType,
22+
TaskPriority, TurboTasksBackendApi, TurboTasksCallApi, backend::CachedTaskTypeArc,
2323
macro_helpers::NativeFunction,
2424
};
2525

@@ -103,15 +103,16 @@ pub trait ExecuteContext<'e>: Sized {
103103
/// Uses hash-based lookup which may return multiple candidates due to hash collisions,
104104
/// then verifies each candidate by comparing the stored `persistent_task_type`.
105105
/// Returns `Some((task_id, task_type))` if a matching task is found, where `task_type` is
106-
/// the existing `Arc<CachedTaskType>` from storage (avoiding a duplicate allocation).
106+
/// the existing `CachedTaskTypeArc` from storage (avoiding a duplicate
107+
/// allocation).
107108
///
108109
/// Accepts exploded components so the caller does not need to box the argument before calling.
109110
fn task_by_type(
110111
&mut self,
111112
native_fn: &'static NativeFunction,
112113
this: Option<RawVc>,
113114
arg: &dyn DynTaskInputs,
114-
) -> Option<(TaskId, Arc<CachedTaskType>)>;
115+
) -> Option<(TaskId, CachedTaskTypeArc)>;
115116
fn debug_get_task_description(&self, task_id: TaskId) -> String;
116117
}
117118

@@ -606,7 +607,7 @@ struct TaskRestoreEntry {
606607
/// Another thread claimed the meta restore; we must wait in Phase 3.
607608
wait_meta: bool,
608609
/// Task type discovered during Phase 1c data restore (used to update task cache in Phase 2).
609-
task_type: Option<Arc<CachedTaskType>>,
610+
task_type: Option<CachedTaskTypeArc>,
610611
/// This thread performed the restore for at least one category (set in Phase 1c).
611612
self_restored: bool,
612613
}
@@ -985,7 +986,7 @@ impl<'e, B: BackingStorage> ExecuteContext<'e> for ExecuteContextImpl<'e, B> {
985986
native_fn: &'static NativeFunction,
986987
this: Option<RawVc>,
987988
arg: &dyn DynTaskInputs,
988-
) -> Option<(TaskId, Arc<CachedTaskType>)> {
989+
) -> Option<(TaskId, CachedTaskTypeArc)> {
989990
if !self.backend.should_restore() {
990991
return None;
991992
}
@@ -1032,14 +1033,14 @@ impl<'e, B: BackingStorage> ChildExecuteContext<'e> for ChildExecuteContextImpl<
10321033
}
10331034

10341035
pub enum TaskTypeRef<'l> {
1035-
Cached(&'l Arc<CachedTaskType>),
1036+
Cached(&'l CachedTaskTypeArc),
10361037
Transient(&'l Arc<TransientTask>),
10371038
}
10381039

10391040
impl TaskTypeRef<'_> {
10401041
pub fn to_owned(&self) -> TaskType {
10411042
match self {
1042-
TaskTypeRef::Cached(ty) => TaskType::Cached(Arc::clone(ty)),
1043+
TaskTypeRef::Cached(ty) => TaskType::Cached((*ty).clone()),
10431044
TaskTypeRef::Transient(ty) => TaskType::Transient(Arc::clone(ty)),
10441045
}
10451046
}
@@ -1056,7 +1057,7 @@ impl Display for TaskTypeRef<'_> {
10561057

10571058
#[derive(Debug)]
10581059
pub enum TaskType {
1059-
Cached(Arc<CachedTaskType>),
1060+
Cached(CachedTaskTypeArc),
10601061
Transient(Arc<TransientTask>),
10611062
}
10621063

@@ -1398,7 +1399,7 @@ impl TaskGuard for TaskGuardImpl<'_> {
13981399
.map(|target| (target, TaskDataCategory::Meta))
13991400
.chain(
14001401
self.iter_cell_dependencies()
1401-
.map(|(target, _key)| (target.task, TaskDataCategory::All)),
1402+
.map(|dep| (dep.cell_ref().task, TaskDataCategory::All)),
14021403
)
14031404
.chain(
14041405
self.iter_collectibles_dependencies()

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::{
2020
},
2121
storage_schema::TaskStorageAccessors,
2222
},
23-
data::CellRef,
23+
data::{CellDependency, CellRef},
2424
};
2525

2626
#[derive(Encode, Decode, Clone, Default)]
@@ -137,17 +137,24 @@ impl UpdateCellOperation {
137137
let mut dependent_tasks: FxIndexMap<TaskId, SmallVec<[Option<u64>; 2]>> =
138138
FxIndexMap::default();
139139
if !skip_invalidation {
140-
let tasks_with_keys =
141-
task.iter_cell_dependents()
142-
.filter_map(|(dependent_cell, key, task)| {
143-
(dependent_cell == cell
144-
&& key.is_none_or(|key_hash| {
145-
updated_key_hashes_set
146-
.as_ref()
147-
.is_none_or(|set| set.contains(&key_hash))
148-
}))
149-
.then_some((task, key))
150-
});
140+
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`.
143+
let (
144+
CellRef {
145+
task: dependent_task,
146+
cell: dependent_cell,
147+
},
148+
key,
149+
) = dep.into_parts();
150+
(dependent_cell == cell
151+
&& key.is_none_or(|key_hash| {
152+
updated_key_hashes_set
153+
.as_ref()
154+
.is_none_or(|set| set.contains(&key_hash))
155+
}))
156+
.then_some((dependent_task, key))
157+
});
151158
for (task, key) in tasks_with_keys {
152159
dependent_tasks.entry(task).or_default().push(key);
153160
}
@@ -276,14 +283,15 @@ impl Operation for UpdateCellOperation {
276283
let mut make_stale = false;
277284
let dependent = ctx.task(dependent_task_id, TaskDataCategory::All);
278285
for key in keys.iter().copied() {
279-
if dependent.outdated_cell_dependencies_contains(&(cell_ref, key)) {
286+
let dep = CellDependency::new(cell_ref, key);
287+
if dependent.outdated_cell_dependencies_contains(&dep) {
280288
// cell dependency is outdated, so it hasn't read the cell yet
281289
// and doesn't need to be invalidated.
282290
// We do not need to make the task stale in this case.
283291
// But importantly we still need to make the task dirty as it should
284292
// no longer be considered as
285293
// "recomputation".
286-
} else if !dependent.cell_dependencies_contains(&(cell_ref, key)) {
294+
} else if !dependent.cell_dependencies_contains(&dep) {
287295
// cell dependency has been removed, so the task doesn't depend on
288296
// the cell anymore and doesn't need
289297
// to be invalidated

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::{
1212
use thread_local::ThreadLocal;
1313
use tracing::span::Id;
1414
use turbo_bincode::TurboBincodeBuffer;
15-
use turbo_tasks::{FxDashMap, TaskId, backend::CachedTaskType, event::Event, parallel};
15+
use turbo_tasks::{FxDashMap, TaskId, backend::CachedTaskTypeArc, event::Event, parallel};
1616

1717
use crate::{
1818
backend::storage_schema::{
@@ -183,7 +183,7 @@ pub struct Storage {
183183
/// This is backed by the TaskCache table in the database.
184184
///
185185
/// LockOrdering: See the comments on [map].
186-
pub task_cache: FxDashMap<Arc<CachedTaskType>, TaskId>,
186+
pub task_cache: FxDashMap<CachedTaskTypeArc, TaskId>,
187187
}
188188

189189
impl Storage {
@@ -253,7 +253,7 @@ impl Storage {
253253
/// Mark a newly allocated task as restored (skip DB queries) and new (include in persistence
254254
/// snapshots). Optionally sets the `persistent_task_type` eagerly so it's available for
255255
/// persistence snapshots without needing to propagate it through `connect_child`.
256-
pub fn initialize_new_task(&self, task_id: TaskId, task_type: Option<Arc<CachedTaskType>>) {
256+
pub fn initialize_new_task(&self, task_id: TaskId, task_type: Option<CachedTaskTypeArc>) {
257257
let mut task = self.access_mut(task_id);
258258
task.flags.set_restored(TaskDataCategory::All);
259259
task.flags.set_new_task(true);
@@ -516,7 +516,7 @@ impl Storage {
516516
// was contended. We defer them until after the map shard lock is released to
517517
// avoid a lock cycle with get_or_create_persistent_task, which takes task_cache
518518
// before map. Allocated lazily on first conflict.
519-
let mut deferred_task_cache_removals: Vec<Arc<CachedTaskType>> = Vec::new();
519+
let mut deferred_task_cache_removals: Vec<CachedTaskTypeArc> = Vec::new();
520520
// SAFETY: We hold the write lock for the duration of iteration.
521521
for bucket in unsafe { shard.iter() } {
522522
// SAFETY: The write lock guard outlives the bucket reference.

0 commit comments

Comments
 (0)