Skip to content

Refactor freelist MPSC queue draining - #881

Open
Matthew Parkinson (mjp41) wants to merge 4 commits into
microsoft:mainfrom
mjp41:feature/freelist-mpscq-link-slots
Open

Matthew Parkinson (mjp41) wants to merge 4 commits into
microsoft:mainfrom
mjp41:feature/freelist-mpscq-link-slots

Conversation

@mjp41

Copy link
Copy Markdown
Member

Summary

  • make FreeListMPSCQ::enqueue() report the empty-to-nonempty transition
  • add concurrent drain_and_reset() and use it consistently from allocator flush and queue cleanup
  • share bounded chain traversal between ordinary dequeue and drain while preserving ordinary dequeue's retained final tail
  • remove redundant queue initialization and quiescent destruction APIs
  • add focused sequential, publication-gap, callback-stop, concurrent producer, storage-reuse, and callable-compatibility tests

Queue semantics

The representation remains message-pointer based: an empty queue has null front and back, and a non-empty queue has front at the first message and back at the final message. Ordinary dequeue still retains the final message.

drain_and_reset() clears and detaches one queue snapshot before invoking callbacks. Producers whose back exchange observes the reset form a replacement queue that is left for a later drain. The operation waits indefinitely for a producer that has exchanged back but has not yet published front or 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

  • final Debug suite: 83/84 on the initial run and both required reruns; only the known perf-lotsofthreads-check 60-second timeout remains
  • func-malloc-fast and func-jemalloc-fast: pass
  • ThreadSanitizer heavy queue stress: 20/20 pass
  • forward-edge and backward-edge focused configurations: 4/4 each
  • available mitigation executables: 5/5 pass; 47 registered variants remain unavailable in the existing local mitigation build configuration
  • exact-name Release codegen comparison: all 12 dequeue instantiations retain their prefetch counts and are slightly smaller; no out-of-line process_chain() symbol or call
  • powered Release comparison: perf-msgpass-fast --smoke median 0.88s to 0.64s; perf-lotsofthreads-fast --smoke median 19.945s to 17.955s
  • final fresh-context implementation review: approved

The 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 --check passes.

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.

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
Comment thread src/snmalloc/mem/freelist_queue.h Outdated
* 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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right. I think this and the original were incorrectly domesticating the head with a link domesticator.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants