turbo-tasks-backend: batch find_and_schedule_dirty using for_each_task_meta - #91497
Merged
Merged
Conversation
Instead of calling find_and_schedule_dirty once per task in a loop, collect up to MAX_COUNT_BEFORE_YIELD jobs into a SmallVec and process them in a single batched call via ctx.for_each_task_meta, allowing the backend to fetch all task data in one operation. Also fixes a pre-existing bug where ctx.get_task_description(task_id) was called on the ExecuteContext instead of the TaskGuard. Co-Authored-By: Claude <noreply@anthropic.com>
…ocks - Replace std::collections::HashMap with FxHashMap (already imported, consistent with the rest of the codebase) - Move the Meta/All performance comment next to the for_each_task_meta call - Combine two consecutive #[cfg(trace_find_and_schedule)] let bindings into a single tuple assignment so both span guards live until the end of the closure (clearer drop semantics, one cfg attribute instead of two) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Tests Passed |
Contributor
Stats from current PR🟢 1 improvement
📊 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
📝 Changed Files (21 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsfailed to diffapp-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsfailed to diffapp-route-ex..ntime.dev.jsDiff too large to display app-route-ex..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route.runtime.dev.jsDiff too large to display app-route.ru..time.prod.jsDiff too large to display pages-api-tu..ntime.dev.jsDiff too large to display pages-api.runtime.dev.jsDiff too large to display pages-turbo...ntime.dev.jsDiff too large to display pages.runtime.dev.jsDiff too large to display server.runtime.prod.jsDiff too large to display 📎 Tarball URL |
Merging this PR will not alter performance
Comparing Footnotes
|
…sk() instead of for_each_task_meta for_each_task_meta holds the TaskLockCounter elevated for the entire duration of its callback. find_and_schedule_dirty_internal drops the task guard and calls ctx.schedule(), which internally calls ctx.task() — incrementing the counter a second time and triggering the "Concurrent task lock acquisition detected" panic in debug builds. Switch to ctx.prepare_tasks() for the parallel prefetch (preserving the batch I/O benefit) and then iterate with individual ctx.task() calls so the counter returns to zero between each task, matching the original one-at-a-time access pattern that schedule() expects. Co-Authored-By: Claude <noreply@anthropic.com>
find_and_schedule jobs are cheaper than aggregation updates — they only read task metadata and optionally schedule a task — so we can process 10x more per process() call (10 000 vs 1 000) before yielding. Co-Authored-By: Claude <noreply@anthropic.com>
Use a standalone literal value (10000) instead of deriving from MAX_COUNT_BEFORE_YIELD, making the constant self-contained. Co-Authored-By: Claude <noreply@anthropic.com>
sokra
marked this pull request as ready for review
March 17, 2026 19:30
lukesandberg
approved these changes
Mar 17, 2026
…or cache-friendly sequential access Co-Authored-By: Claude <noreply@anthropic.com>
Release the TaskLockCounter before calling prepared_task_callback instead of after. This ensures the counter is 0 when the callback runs, so callbacks that drop their task guard and then call ctx.task() (like find_and_schedule_dirty_internal → ctx.schedule()) no longer trigger the "Concurrent task lock acquisition detected" panic. This lets find_and_schedule_dirty use for_each_task_meta directly, reverting the workaround from b576d8e that bypassed for_each_task_meta and called ctx.task() individually after a manual ctx.prepare_tasks(). The for_each_task callback now uses acquire() instead of reacquire() since the counter is guaranteed to be 0 at callback entry. reacquire() is removed as it has no remaining callers.
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?
Batch-process
find_and_schedule_dirtyinaggregation_update.rsby collecting all queued jobs (up toFIND_AND_SCHEDULE_BATCH_SIZE= 10 000) into aSmallVecand pre-fetching their task metadata with a batchedctx.for_each_task_meta(...)call.Why?
find_and_schedulecan accumulate thousands of tasks during invalidation cascades. The previous implementation issued onectx.task(...)call per task insideprocess(), serializing backing-storage fetches one at a time.ctx.for_each_task_metatriggers a batched fetch of all task metadata from the backing store: keys are sorted by hash for cache-friendly sequential access to the storage layer. The callback is invoked per-task once data is ready, with the task guard handed directly to the callback — no second lock acquisition needed.The batch limit is set to 10 000, because find-and-schedule jobs are cheap (metadata read + optional schedule) compared to aggregation-update jobs, so yielding less often is safe and beneficial.
How?
fn process—find_and_schedulebranch:drain(..FIND_AND_SCHEDULE_BATCH_SIZE)that collects up to 10 000FindAndScheduleJobstructs into aSmallVec, then callsfind_and_schedule_dirtyonce with the whole batch.fn find_and_schedule_dirty:task_id: TaskIdtojobs: SmallVec<[FindAndScheduleJob; 4]>.ctx.for_each_task_meta(...)to batch-prefetch all task metadata (sorted by hash for cache-friendly access) and process each task in the callback.prepare_tasks_with_callbackfix:TaskLockCounterbefore callingprepared_task_callbackinstead of after. This ensures the counter is 0 when the callback runs, so callbacks that drop their task guard and then callctx.task()(likefind_and_schedule_dirty_internal→ctx.schedule()) no longer trigger the "Concurrent task lock acquisition detected" panic in debug builds.for_each_tasknow usesacquire()instead ofreacquire()since counter is guaranteed 0 at callback entry.reacquire()is removed.Cleanup:
FxHashMap(already imported) instead ofstd::collections::HashMap.#[cfg(trace_find_and_schedule)]let bindings for clarity.FIND_AND_SCHEDULE_BATCH_SIZE = 10_000as a self-contained constant (not derived fromMAX_COUNT_BEFORE_YIELD).