review

Author: lukesandberg

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

@@ -2919,20 +2919,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
 
                             if this.should_evict() {
                                 if !check_idle_ended!() {
-                                    let evict_span = tracing::info_span!(
-                                        parent: background_span.id(),
-                                        "evict tasks",
-                                        full = tracing::field::Empty,
-                                        data_and_meta = tracing::field::Empty,
-                                        data_only = tracing::field::Empty,
-                                        meta_only = tracing::field::Empty,
-                                    );
-                                    let _guard = evict_span.enter();
-                                    let counts = this.storage.evict_after_snapshot();
-                                    evict_span.record("full", counts.full);
-                                    evict_span.record("data_and_meta", counts.data_and_meta);
-                                    evict_span.record("data_only", counts.data_only);
-                                    evict_span.record("meta_only", counts.meta_only);
+                                    this.storage.evict_after_snapshot();
                                 }
                             }
 

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

@@ -388,7 +388,7 @@ impl Storage {
     /// - `No`: skip
     ///
     /// Must be called when NOT in snapshot mode (i.e., after `end_snapshot()`).
-    pub fn evict_after_snapshot(&self) -> EvictionCounts {
+    pub fn evict_after_snapshot(&self) {
         let span = tracing::trace_span!(
             "evict_after_snapshot",
             task_cache = tracing::field::Empty,
@@ -489,7 +489,6 @@ impl Storage {
         span.record("data_only", totals.data_only);
         span.record("meta_only", totals.meta_only);
         span.record("skipped", skipped);
-        totals
     }
 }
 

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

@@ -500,6 +500,9 @@ impl TaskStorage {
         }
 
         // This is common after a round of eviction we end up with tasks with only transient state
+        // There is no need to search for it, we can just assume any task in this state is preserved
+        // for a reason.  NOTE: new tasks have the restored flags set as part of construction so the
+        // only way for a task to end up in this situation is through eviction
         if !flags.data_restored() && !flags.meta_restored() {
             return (
                 key_evictability,
@@ -510,7 +513,8 @@ impl TaskStorage {
         // Back off if another thread is currently restoring this task's data from
         // disk. Without this check, eviction could clear data that was already
         // determined to be "restored" by the restoring thread (which released the
-        // lock to do I/O), causing the restoring thread to skip re-reading it.
+        // lock to do I/O), causing the restoring thread to skip re-reading it.  Instead we respect
+        // in flight restoration
         if flags.meta_restoring() || flags.data_restoring() {
             return (
                 key_evictability,
@@ -526,6 +530,8 @@ impl TaskStorage {
             && !flags.data_modified_during_snapshot()
             && self.transient_cell_data().is_none_or(|m| m.is_empty())
             // If transient tasks depend on our cells or output we cannot be evicted
+
+            // TODO: this is the most expensive part of the scan since there can be many cell and output dependents as well as many cells
             && self
                 .cell_dependents()
                 .is_none_or(|cd| !cd.iter().any(|(_, _, t)| t.is_transient()))
@@ -543,11 +549,12 @@ impl TaskStorage {
         let meta_evictable = flags.meta_restored()
             && !flags.meta_modified()
             && !flags.meta_modified_during_snapshot()
+            && self.get_output().is_none_or(|o| !o.is_transient())
+            // TODO: this is the most expensive part of the scan since there can be many dependents and uppers
             && self
                 .collectibles_dependents()
                 .is_none_or(|d| !d.iter().any(|(_trait_id, task)| task.is_transient()))
-            && !self.upper().iter().any(|(k, _)| k.is_transient())
-            && self.get_output().is_none_or(|o| !o.is_transient());
+            && !self.upper().iter().any(|(k, _)| k.is_transient());
 
         // === Combined decision ===
         (

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

@@ -202,7 +202,10 @@ pub(crate) static VALUES: Lazy<Box<[&'static ValueType]>> = Lazy::new(|| {
             .copied()
             .collect(),
         |v, mut id| {
-            assert!(id.get() & SESSION_STATEFUL_BIT == 0);
+            assert!(
+                id.get() & SESSION_STATEFUL_BIT == 0,
+                "SESSION_STATEFUL_BIT already set? do we have more than 32k value types"
+            );
             if v.is_session_stateful {
                 id |= SESSION_STATEFUL_BIT;
             }