Refactor freelist MPSC queue draining - #881
Open
Matthew Parkinson (mjp41) wants to merge 4 commits into
Open
Matthew Parkinson (mjp41) wants to merge 4 commits into
Matthew Parkinson (mjp41) wants to merge 4 commits into
Conversation
Expose empty-to-nonempty enqueue transitions and add a concurrent drain-and-reset operation. Share bounded chain processing with ordinary dequeue while preserving its retained tail semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75ccdc6c-2024-4d81-b22f-f1b52fbe29cf
Rename the focused test's namespace-scope freelist key so instantiated allocator parameters named key do not trigger C4459 under warnings-as-errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75ccdc6c-2024-4d81-b22f-f1b52fbe29cf
| * Where necessary, dequeue exposes two domesticator callbacks and is careful | ||
| * to use one for the front value and the other for pointers read from the | ||
| * queue itself. Draining uses its queue domesticator for both the front | ||
| * value and links in the chain. Specifically, |
Contributor
There was a problem hiding this comment.
Hm. It's been a while, but I thought the two of these were there because pointers to freelist heads and intra-list link pointers were different types?
Member
Author
There was a problem hiding this comment.
I think you're right. I think this and the original were incorrectly domesticating the head with a link domesticator.
Member
Author
There was a problem hiding this comment.
Fixed in d7f262e. drain_and_reset now takes distinct head and queue domesticators, matching dequeue. CoreAllocator::flush uses the trusted annotating conversion when QueueHeadsAreTame and validates the head otherwise; decoded message links always use the queue domesticator. Added focused separation, rejected-head, and rejected-successor tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75ccdc6c-2024-4d81-b22f-f1b52fbe29cf
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 75ccdc6c-2024-4d81-b22f-f1b52fbe29cf
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
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.
Summary
FreeListMPSCQ::enqueue()report the empty-to-nonempty transitiondrain_and_reset()and use it consistently from allocator flush and queue cleanupQueue semantics
The representation remains message-pointer based: an empty queue has null
frontandback, and a non-empty queue hasfrontat the first message andbackat the final message. Ordinary dequeue still retains the final message.drain_and_reset()clears and detaches one queue snapshot before invoking callbacks. Producers whosebackexchange observes the reset form a replacement queue that is left for a later drain. The operation waits indefinitely for a producer that has exchangedbackbut has not yet publishedfrontor its predecessor link; this preserves the previously agreed fork/interrupted-producer tradeoff.The shared
process_chain()primitive processes objects strictly before a fixed address bound and returns the first unprocessed object. Dequeue retains that object; drain retries an unpublished link until it reaches and consumes the captured target.Validation
perf-lotsofthreads-check60-second timeout remainsfunc-malloc-fastandfunc-jemalloc-fast: passprocess_chain()symbol or callperf-msgpass-fast --smokemedian 0.88s to 0.64s;perf-lotsofthreads-fast --smokemedian 19.945s to 17.955sThe repository-provided clang-format 15 binary is x86-64 and cannot run on this ARM64 host; the five changed source files were formatted and verified with clang-format 16, and
git diff --checkpasses.Deferred work
This PR does not implement allocator sleeping or retirement, pool scavenging or other pool policy, retained-tail retirement, wake counters or hints, or the N+1 benchmark. Those remain separate allocator-lifecycle work.