-
Notifications
You must be signed in to change notification settings - Fork 30.3k
Add a new keyspace for the reverse task mappiong to avoid storing task-types multiple times #88904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: canary
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| in_progress_cells: AutoMap<CellId, InProgressCellState>, | ||
|
|
||
| #[field(storage = "direct", category = "data")] | ||
| #[field(storage = "direct", category = "transient")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // (persistent_task_type is transient and not restored from serialized data) | ||
| if task.get_persistent_task_type().is_none() && !task_id.is_transient() { | ||
| let tx = self.get_tx(); | ||
| // Safety: `tx` is a valid transaction from `self.backend.backing_storage`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failing test suitesCommit: e929b37 | About building and testing Next.js
Expand output● after during server shutdown - custom server › waits for after callbacks when the server receives SIGTERM
Expand output● Custom routes › no-op rewrite › should not error for no-op rewrite and auto export dynamic route ● Custom routes › should load custom routes when only one type is used › development mode › should work with just headers ● Custom routes › should load custom routes when only one type is used › development mode › should work with just rewrites ● Custom routes › should load custom routes when only one type is used › development mode › should work with just redirects
Expand output● config-output-export › should work with getStaticProps and without revalidate ● config-output-export › should error with getServerSideProps without fallback ● config-output-export › should error with getStaticPaths and fallback true ● config-output-export › should error with getStaticPaths and fallback blocking ● config-output-export › should work with getStaticPaths and fallback false
Expand output● filesystem-caching with cache enabled › should cache or not cache loaders ● filesystem-caching with cache enabled › should allow to change files while stopped (RSC change) ● filesystem-caching with cache enabled › should allow to change files while stopped (RCC change) ● filesystem-caching with cache enabled › should allow to change files while stopped (Pages change) ● filesystem-caching with cache enabled › should allow to change files while stopped (rename app page) ● filesystem-caching with cache enabled › should allow to change files while stopped (next config change) ● filesystem-caching with cache enabled › should allow to change files while stopped (env var change) ● filesystem-caching with cache enabled › should allow to change files while stopped (RSC change, RCC change, Pages change, rename app page, next config change, env var change) |
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
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: **432 kB** → **432 kB** ✅ -32 B82 files with content-based hashes (individual files not comparable between builds) Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
|
0c1d348 to
9e97759
Compare
aebd28d to
9562e78
Compare
…k-types multiple times
9562e78 to
e929b37
Compare
| /// values. | ||
| /// | ||
| /// Returns the key bytes if an entry with matching hash and value is found. | ||
| pub fn lookup_key_by_hash_and_value( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some tracing to this method similar to the other db read methods
| task_meta: factory(KeySpace::TaskMeta), | ||
| task_data: factory(KeySpace::TaskData), | ||
| task_cache: factory(KeySpace::TaskCache), | ||
| task_id_to_task_type_hash: factory(KeySpace::TaskIdToTaskTypeHash), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we store that in the task_data to avoid the extra key space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i was thinking that too actually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it makes restoring more complex since the task_type would need to be a HashOrTaskType enum or something but it works i think

Add reverse lookup for task types by task ID
What?
Adds a reverse lookup mechanism to efficiently retrieve task types from task IDs in the Turbo persistence layer.
Why?
Improves performance by allowing direct lookup of task types when only the task ID is available, avoiding the need to deserialize the entire task.
How?
lookup_key_by_hash_and_valuemethod to the persistence layer that finds keys by their hash and confirms matches by comparing valuespersistent_task_typetransient to avoid redundant storageFixes #