Repository navigation
perf(sort): restore lock-free unpark via ArcSwap instead of Mutex - #396
Merged
Merged
Conversation
The streaming-sort port needed a runtime-settable pool main-thread handle (set_main_thread re-targets unpark from the chain-build thread to the framework worker that actually drives the fused sort). The first cut wrapped it in a parking_lot::Mutex<Thread>, which serialized the per-block unpark hot path on a single shared lock every worker contends on — a cache-line bounce that regresses parity with the lock-free baseline on BOTH standalone and streaming sort, worse as threads scale. Replace it with arc_swap::ArcSwap<Thread>: the 16 unpark sites become wait-free .load().unpark() (no mutual exclusion, matching the bare-Thread baseline's scaling); set_main_thread becomes a lock-free store. Correctness against the re-target race is identical to the Mutex form (neither guards a concurrent live re-target) and rests on the documented one-shot-at-drive-entry ordering + the condition-checked park loop, not mutual exclusion — see the set_main_thread doc. Audited all 3 set_main_thread call sites (into_slot_setup ×3, each one-shot before the first park; standalone sort needs none — same-thread pool). Adds a unit test that set_main_thread re-targets the handle to the calling thread.
nh13
force-pushed
the
feat-runall-05-restore-lockfree-unpark
branch
from
June 15, 2026 04:40
990ecfa to
e9c7305
Compare
nh13
added a commit
that referenced
this pull request
Jun 18, 2026
The streaming-sort port needed a runtime-settable pool main-thread handle (set_main_thread re-targets unpark from the chain-build thread to the framework worker that actually drives the fused sort). The first cut wrapped it in a parking_lot::Mutex<Thread>, which serialized the per-block unpark hot path on a single shared lock every worker contends on — a cache-line bounce that regresses parity with the lock-free baseline on BOTH standalone and streaming sort, worse as threads scale. Replace it with arc_swap::ArcSwap<Thread>: the 16 unpark sites become wait-free .load().unpark() (no mutual exclusion, matching the bare-Thread baseline's scaling); set_main_thread becomes a lock-free store. Correctness against the re-target race is identical to the Mutex form (neither guards a concurrent live re-target) and rests on the documented one-shot-at-drive-entry ordering + the condition-checked park loop, not mutual exclusion — see the set_main_thread doc. Audited all 3 set_main_thread call sites (into_slot_setup ×3, each one-shot before the first park; standalone sort needs none — same-thread pool). Adds a unit test that set_main_thread re-targets the handle to the calling thread.
This branch had an error being deployed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Stacked PR (stack position 05) of the #330 unified-pipeline landing. Targets the long-lived integration branch
feat-runall(notmain); #395 (streaming-sort API port) is its merged predecessor. One commit.The streaming-sort port needs a runtime-settable pool main-thread handle (
set_main_threadre-targetsunparkfrom the chain-build thread to the framework worker that actually drives the fused sort). The first cut wrapped it in aparking_lot::Mutex<Thread>, which serialized the per-blockunparkhot path on a single shared lock every worker contends on — a cache-line bounce that regresses parity with the lock-free baseline on both standalone and streaming sort, worse as threads scale.Replace it with
arc_swap::ArcSwap<Thread>: the 16unparksites become wait-free.load().unpark()(no mutual exclusion, matching the bare-Threadbaseline's scaling);set_main_threadbecomes a lock-free store. Correctness against the re-target race is identical to theMutexform (neither guards a concurrent live re-target) and rests on the documented one-shot-at-drive-entry ordering + the condition-checked park loop, not mutual exclusion — see theset_main_threaddoc.Audited all 3
set_main_threadcall sites (into_slot_setup×3, each one-shot before the first park; standalone sort needs none — same-thread pool). Adds a unit test thatset_main_threadre-targets the handle to the calling thread.Expected CI (documented): this PR will be red on the 8 rejects-header tests (
test_*_rejects_has_bgzf_eof,test_correct_command_rejects_streaming_threaded_integrity,test_duplex_command_*rejects*) — a pre-existingfeat-runall-base failure family asserting the #332 "rejects BAM carries input@HDverbatim" contract, exposed once the streaming-sort API made the suite compile (at stack-03/#395). They are fixed cumulatively: 7 go green at the next PR (fix(correct), stack-06), the last (codec) at stack-07. Any CI failure outside that 8-test set is a real regression. This commit itself is aworker_pool.rs-only perf change and introduces no new failures.