turbo-tasks: fix hashed cell mode crash on task error (re-land #91576)#92108
Merged
Conversation
Contributor
Tests Passed |
Merging this PR will not alter performance
Comparing Footnotes
|
lukesandberg
approved these changes
Mar 31, 2026
Contributor
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📎 Tarball URL |
af0fbdf to
fc6911e
Compare
lukesandberg
approved these changes
Apr 1, 2026
505df5f to
9ec8687
Compare
Co-Authored-By: Claude <noreply@anthropic.com>
9ec8687 to
85ab7e0
Compare
…d of removed resolve_strongly_consistent() Co-Authored-By: Claude <noreply@anthropic.com>
sokra
commented
Apr 7, 2026
sokra
commented
Apr 7, 2026
…dianness, improve Code persistence - Clean up cell_data_hash entries in task_execution_completed_cleanup for removed cells - Extract duplicated hash-update logic into update_cell_data_hash() helper - Use to_le_bytes() instead of to_ne_bytes() for cross-platform hash consistency - Add cell_persisted() to Code to avoid triple-cell indirection - Change Code.mappings to Arc<Vec<Mapping>> to reduce clone cost - Replace rand::random with AtomicU32 counter in hashed_cell_mode test Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Re-lands #91576 ("turbo-tasks: add hashed cell mode for hash-based change detection without cell data"), which was reverted in #92103 due to a
FATALcrash in thefilesystem-cachetest suite.Includes a bug fix on top: in
task_execution_completed_prepare, skip updatingcell_type_max_indexwhen the task completed with an error.Also adds a
CellHash = [u8; 16]type alias (requested in review) used throughout the hash pipeline.Why?
The original feature (
serialization = "hash"onFileContentandCode) stores a hash of the cell data instead of the full serialized value. On session restore, the hash is used to detect whether cell content has changed without needing the full data in memory. This avoids a large persistent cache size increase.The bug that caused the revert: When a task fails partway through re-execution (before recreating all the cells from its previous run),
cell_countersonly reflects the partially-executed state. The old code used those partial counters to updatecell_type_max_index, removing entries for cell types that were not yet created at the point of failure. This caused downstream tasks that still held cell dependencies from the previous successful run to hit a hard "Cell no longer exists" error.Concrete failure path in
filesystem-cache rename app pagetest:get_app_page_entryruns for/remove-me/page, creating twoFileContentcells (indices 0 and 1).cell_type_max_index[FileContent] = 2is persisted.app/remove-me→app/add-me), dirtying the task.get_app_page_entryfails atconfig.await?(the loader tree errors because the directory is gone) — before anyFileContent::cell()calls.cell_countershas noFileContententry → old code removedcell_type_max_index[FileContent].parsetask tries to readFileContentcell 1 fromget_app_page_entry→cell_type_max_indexisNone→ "Cell no longer exists" panic → FATAL error.Why it didn't crash before
serialization = "hash":FileContentwas previously serializable, soparseread stale cell data directly frompersistent_cell_data, whichtask_execution_completed_cleanupalready preserves on error. Withserialization = "hash", data is transient — readers fall back tocell_type_max_indexfor range validation, where a staleNonecaused the crash.How?
Core feature:
serialization = "hash"cell modeSerializationMode::Hashvariant inturbo-tasks-macros— marks a value type as non-serializable but stores aDeterministicHashof the cell data for change detection.VcCellHashedCompareMode<T>cell mode: compares values viaPartialEqwhen available, falls back to hash comparison when transient data has been evicted.hashed_compare_and_update/hashed_compare_and_update_with_shared_referenceonCurrentCellRefcompute and pass content hashes through the update pipeline.update_celluses hash-based comparison to skip invalidation when the old cell data is unavailable but the hash matches.cell_data_hash: AutoMap<CellId, CellHash>field in task storage persists hashes across sessions.cell_data_hashentries are cleaned up intask_execution_completed_cleanupalongside cell data removal.CellHash = [u8; 16]type alias keeps alignment at 1 byte to avoid padding growth inAutoMap/LazyFieldenum variants.to_le_bytes) for cross-platform cache portability.Bug fix: preserve
cell_type_max_indexon task errorIn
task_execution_completed_prepare, guard thecell_type_max_indexupdate block withif result.is_ok(). This mirrors the existingtask_execution_completed_cleanupbehavior that already skips cell data removal whenis_erroris true, keepingcell_type_max_indexconsistent with the preserved transient cell data.Applied to
FileContentandCodeFileContentusesserialization = "hash"— full content is persisted via a separatePersistedFileContenttype when needed (e.g., inDiskFileSystem::write).Codeusesserialization = "hash"withArc<Vec<Mapping>>for cheap cloning.Code::cell_persisted()creates aPersistedCodecell directly and returnsVc<Code>viaPersistedCode::to_code(), avoiding an intermediate hash-mode cell.Other improvements
DeterministicHashimpls forSmallVecand().Xxh3Hash128Hasher::finish_bytes()method returning[u8; 16].hash = "manual"option on#[turbo_tasks::value]to opt out of auto-derivingDeterministicHash.Note: The shutdown hang and cache poisoning fixes that were previously on this branch have been merged separately via #92254.
Test plan
test/e2e/filesystem-cache/filesystem-cache.test.tspasses (all 17 tests)turbopack/crates/turbo-tasks-backend/tests/hashed_cell_mode.rsintegration test verifies hash-based change detection: value changes trigger invalidation, equal values (same hash) do notcargo checkpasses forturbo-tasks,turbo-tasks-backend,turbo-tasks-fs,turbopack-core,turbopack-ecmascript