Repository navigation
Use a resizable receive ring with one inline descriptor - #17
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thweetkomputer
marked this pull request as ready for review
October 19, 2026 13:04
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.
Replace Connection's deque with a receive-descriptor ring that stores its first descriptor inline. Depth-zero/one traffic performs no queue heap allocation. Overflow allocates 8 slots, then doubles when full; sustained low occupancy halves heap storage and eventually returns to the inline slot. Current shared-dataset cross-over results are recorded below; the historical fresh-fill initial-ring results remain separately labeled.
The measured throughput changes are mostly small: 10M scenario medians stay within ±0.36%. 1B SET/640 improves in all three pairs (+1.57%, +0.93%, +1.55%; median +1.55%), and 1B GET/640 has a median +0.41%. Tail latency is not uniformly better: 1B GET/320 p99.99 is higher in all three pairs (+2.84%, +2.19%, +9.95%); 10M GET/320 p99.99 has a +10.85% median change with mixed signs. These results do not support a blanket speedup or a no-regression claim.
The first 1B SET/320 pair is −11.83% QPS; the next two are +0.56% and +1.04%, and all are retained. The unchanged control itself moves from 903,807 QPS in the first round to 795,615 and 794,747 later, while the candidate is 796,890, 800,032 and 803,029. This exposes a substantial stage/time-dependent effect; the experiment does not isolate its cause or justify attributing the first-pair drop solely to the queue. Shared-data history and ordinary time effects remain confounded.
Current implementation
InlineRingQueue<T, InlineCapacity>inbycorf/base/inline_ring_queue.h;Connection::received_buffers_directly usesInlineRingQueue<ReceivedBuffer, 1>. The receive descriptor stays inconnection.h; no separate receive-queue wrapper header or production alias is needed. Inline capacity is a compile-time positive power of two (up to 2^29), preserving masked indexing. Heap growth starts atmax(8, 2 * InlineCapacity). The utility requires trivially copyable/destructible values with nonthrowing copy construction; default construction is not required. It is single-threaded and does not manage resources referenced by its values.capacity_identifies the active member; transitions start the new member lifetime. Growth allocates/copies before releasing the old array; returning inline saves the pointer, copies live values, then deletes the array. Copying is statically required not to throw. Empty slots have no active descriptor; placement construction initializes only enqueued/copied elements. Both unused inline storage and unused heap slots avoid descriptor initialization. Explicit trivial-destructor invariants make pop, clear, overwrite and array destruction safe.clear(), so close does not allocate or relocate through shrink.Evaluated and excluded: single-slot push/pop fast paths
A separate candidate added
[[likely]]single-slot push/pop branches and outlined all remaining single-slot operations intoPushSlow/PopSlow. GCC 13 x86-64 caller code shrank from 176 to 62 bytes for push and from 85 to 29 bytes for pop, with unchanged object layout. It was compared directly against this PR's3cc3586on the same retained pair: 32 client threads, GET/SET, 320/640 connections, 10M/1B keys, three paired rounds and 48 formal windows. The 1B dataset was reused; 10M was loaded once. No previous measurement rows were pooled.Paired median QPS changes ranged from -0.50% to +0.23%, without a clear throughput gain. 10M GET/320 p99.99 worsened in all three pairs: 1.847→6.335 ms, 3.999→5.695 ms, 4.015→6.175 ms (median paired change +53.80%). 10M GET/640 p99.99 worsened in two of three pairs (median +47.20%). Both versions passed sanitizer/allocation checks; the candidate also passed five runtime checks. The experiment does not identify the microscopic cause or exclude storage-history and VM effects. This candidate was rejected and is not included in this PR;
3cc3586retains its original push/pop implementation. Smaller caller code alone is not evidence of improved throughput or tail latency.Layout and allocation checks
Native GCC 13.3 x86-64, using the retained server's actual linked mimalloc library:
c53adc0)Thus the measured depth-one allocation blocks total 288→192 bytes per connection, with one fewer allocation. This excludes backend receive buffers, allocator/page overhead and other connection state; it is not a measured RSS change. The original deque version's Connection was 232 bytes.
The first 64 bytes of Connection retain the merged receive-control layout. The queue still begins at offset 96; its inline descriptor and mutable counters fit in the first 32 bytes. The heap pointer shares the inline-storage union, and counters begin at queue offset 16. A 1,000-allocation mimalloc probe returned 64-byte-aligned 192-byte blocks. This alignment is allocator-dependent, not an
alignof(Connection)guarantee. Validate actual addresses, generated accesses and paired throughput/tails before claiming a cache-line performance improvement. No Task, RemoteWork, storage-awaitable or Lavik layout edits are included.Queue-depth evidence
A separate diagnostic build based on
c53adc0profiled the previous workloads on the retained Azure pair: 10M/1B keys, GET/SET, 320/640 total connections, 32 client threads. Across all 8 formal windows and 347,465,750 enqueues, every post-enqueue depth and every per-connection peak was 1; every pop emptied the queue. Capacity stayed 8 and no growth/shrink followed the initial allocation. Counts matched all 3,840 formal connection lifetimes and enqueue/dequeue/close conservation checks. The diagnostic counters were outside Connection and did not change its layout.These are event-weighted observations of the existing non-pipelined workload, not production-wide or time-weighted occupancy estimates. High pipeline depth, large requests and bursts can still require the dynamic queue. Diagnostic-run QPS is not an optimization comparison.
Current correctness validation
The latest header-only organization change (
7e1eebb) removesreceived_buffer_queue.h, restoresReceivedBuffertoconnection.h, and directly instantiates the generic queue on Connection. Its fresh production build, backend smoke and five runtime checks pass; queue/Connection sizes and offsets are unchanged. The queue algorithm is unchanged. Performance tables below were acquired on3cc3586before this header cleanup; they are not a new benchmark of7e1eebb.-O3 -march=x86-64-v2probe sizes: push 176 B, pop 85 B, front 28 B, Grow 321 B, Shrink 415 B. Code size is not a throughput measurement.3cc3586using the shared-dataset protocol below. Historical results are retained separately, including unfavorable results.Current benchmark — shared-dataset cross-over (
3cc3586)Both arms freshly rebuilt with Lavik
2ae16e9b86ce274a7705542d29c63e3c7a79fb2f: control Bycorf86d686724ddd1c5617fb1b62f8131e05571e8423, candidate3cc3586ab421f567ab1cae97a5c53841d5c8a271.At the user's request, load each size only once in a separate baseline process, checkpoint and exit, then restart the two versions on that same persisted dataset in AB/BA/AB order. Both sizes checkpoint on every switch; all formal starts verify exact DBSIZE and five sampled value lengths. An initial attempt stopped before candidate execution because copying the ELF omitted its executable mode. Its four baseline windows are archived separately and excluded. Permissions were fixed without changing binary hashes; the existing 10M dataset and those prior SET updates were retained. SET changes and storage history persist between cells, so sequence only partially balances carryover. This is distinct from the historical fresh-fill protocol below.
All nine CI jobs passed for
3cc3586, including amd64/arm64, ordinary/GCC Debug, bypass OFF/ON and formatting.Retained Korea Central pair; 16 server logical CPUs, 32 client threads, 320/640 connections, 10M/1B keys, GET/SET, 1,024-byte values, pipeline 1; 30-second 10M / 60-second 1B windows. Six 10M warmups, two one-time fills and 48 formal windows. No historical rows pooled. All formal windows passed configuration, duration, zero-error/miss and JSON/text checks; 806 selected server files and 224 cross-host JSON pairs verified. The original 494 source-file identities were also checked. Background controls restored and both VMs kept running.
Median of three paired percentage changes; positive QPS is better, positive latency is worse. The contrast covers the complete PR, not union/raw-pointer changes in isolation.
All current paired changes
Server/client snapshots recorded zero steal, softnet drops/time-squeeze and interface error/drop increments. Aggregate execution ranges were 92.3–99.5% across 16 server logical CPUs and 51.7–62.6% across 32 client CPUs; snapshots include setup/exit overhead and do not exclude individual core limits or VM noise.
Three pairs do not establish statistical equivalence or a universal improvement. Cache/storage history, restarts and ordinary VM variation remain limitations.
Completed benchmark — initial ring revision
Same prior workload protocol on the retained Korea Central pair: L16aos_v4 server (16 logical CPUs, cores 0–15) and F32als_v6 client (32 threads on cores 0–31). Kernel TCP / SPDK storage; pipeline 1; 1,024-byte values; 320/640 total connections; 10M/1B keys; GET/SET; fresh fills. Three paired rounds in AB/BA/AB order, with 30-second 10M and 60-second 1B measurement windows. All 48 formal windows / 12 cells completed. Historical compact-layout or 16-thread results are not pooled.
Both arms were freshly rebuilt with Lavik
2ae16e9b86ce274a7705542d29c63e3c7a79fb2f. Control: merged Bycorf86d686724ddd1c5617fb1b62f8131e05571e8423. Candidate production code:6da7a471d13b8ab1069906ff46abd8e543c6ff57. The tables below describe only6da7a471d13b8ab1069906ff46abd8e543c6ff57, which allocated its first 8-slot array lazily. These historical results predate direct-front, outlined slow-path, consume/close and inline-storage changes. The current3cc3586shared-dataset experiment is reported separately above; do not apply these historical QPS or latency changes to the current implementation.Each entry below is the median of three paired percentage changes,
(ring / deque - 1) × 100. Positive QPS is better; positive latency is worse. Percentiles are full-window measurements, not averages of per-second percentiles.All paired changes, including unfavorable rounds
Validation and limits
std::deque, wrapped growth, delayed contraction with queued entries, partial offsets, moves, clear/reuse; Linux ASan/UBSan and local ARM64 UBSan passed.0d1335crevision, including amd64/arm64, ordinary/GCC Debug, bypass OFF/ON, and formatting.Three pairs describe this experiment, not statistical equivalence or a universal result. The 10M GET/640 tail signal needs investigation before treating this as a no-regression optimization. These standard workloads do not isolate worst-case resize latency or measure resize frequency, so the tail changes cannot be attributed specifically to resizing or cache-line effects.