Skip to content

[Storage] Optimize IOPS for RAID-0 NVMe disks - #2018

Open
Badrish Chandramouli (badrishc) wants to merge 79 commits into
mainfrom
badrishc/optimize-device-iops
Open

[Storage] Optimize IOPS for RAID-0 NVMe disks#2018
Badrish Chandramouli (badrishc) wants to merge 79 commits into
mainfrom
badrishc/optimize-device-iops

Conversation

@badrishc

@badrishc Badrish Chandramouli (badrishc) commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Description of Change

Garnet's Native storage device (the C++ shim over Linux libaio / io_uring, and IOCP on
Windows) could not keep a fast NVMe array busy: on an 8×NVMe RAID-0 whose fio 4K random-read
ceiling is 8.24 M IOPS, the device topped out around 4.1 M and disk-served RESP GET
throughput was correspondingly capped. This PR removes the submit/completion bottlenecks so the
device reaches fio parity and the Tsavorite KV layer follows.

Results on the reference host (8×NVMe RAID-0, 100% random reads served from disk):

layer before after vs fio ceiling
raw device (Device.benchmark, libaio) 4.14 M 8.09–8.23 M ~100%
raw device (Device.benchmark, io_uring) 8.41–8.45 M ~100%
Tsavorite KV (KV.benchmark, disk-bound) 6.3 M ~77%

The KV gap is Tsavorite managed per-op CPU (hash lookup, pending context, completion dispatch),
not the device. The three benchmarks use different datasets and configurations, so their absolute
numbers are not directly comparable.

Key changes:

  • Per-submitter sharding of in-flight tracking. A single global pending counter plus a shared
    free-slot queue were the dominant cost at high IOPS. In-flight is now tracked per submitter
    thread across NumShards cache-line-padded shards, each with its own completion-slot free list.
    activeShards is maintained exactly on the 0→1 / 1→0 transitions so the per-thread throttle
    divisor tracks live submitters with no global counter and no reconciliation.
  • Ring count decoupled from drainer count. io_uring is ring-starved below submitter
    concurrency; previously rings were tied to the completion-thread count. --device-io-contexts
    now sets the ring count independently, and its smart default
    (max(completion-threads, min(2 × ProcessorCount, 64))) sizes rings to the hardware with no user action.
  • Affine inline drain. TryCompleteMine() drains only the calling thread's own ring instead of
    walking every ring, cutting syscalls N× and removing cross-context ring-lock contention.
    AllocatorBase.AsyncGetFromDisk's throttle-wait uses it.
  • Multiple completion drain threads (--device-completion-threads, default 4). A single
    drainer convoys on the completion-signal path and collapses throughput under load.
  • libaio reservation sized against the global fs.aio-max-nr budget instead of blindly
    requesting a deep ring, so many devices (e.g. cluster aux logs) still start on a stock host.
    The per-device budget ceiling runs last and caps aggregate in-flight along with it, so a stock
    65536 budget halves the default 4096 throttle (~9% on libaio disk serving). A disk-serving host
    should size fs.aio-max-nr / --device-aio-max-devices ≥ --device-throttle-limit, i.e.
    fs.aio-max-nr ≥ 131072 for the defaults.
  • Opt-in io_uring SQPOLL (--device-uring-sqpoll), one poll thread per ring so submission stays
    parallel across rings.
  • Buffer pool depot stripes sized from the machine. The shared overflow depot in
    BufferPool.OriginReturn.cs had a fixed 8 stripes, so on a large box the threads that spill to
    it serialize on those 8 locks. The stripe count now comes from the same ConcurrencySharding
    formula (floor 8, cap 64, rounded to a power of two so the existing mask indexing still
    applies). No other pool constant changes, so the byte budget still bounds retained memory.
  • C ABI exception firewall. Every native entry point catches C++ exceptions and maps them to a
    return code; the managed callback never lets an exception escape into the native dispatch loop
    (which would silently kill the drainer thread and hang all subsequent IO). Reporting that failure
    is itself guarded, since a host-supplied logger can throw.
  • Lock-free disposal safety. Dispose() publishes disposedFlag and drains in-flight; submit
    and lease paths bump their shard counter then re-check the flag (Dekker), so the native handle
    cannot be freed under an in-flight IO or a non-IO native call. ReadAsync / WriteAsync take a
    second, independently balanced lease around the native call: the IO's own bump is dropped by the
    completion callback on a drainer thread, which can fire before the submitting frame has left
    native code.
  • Device creation is exception-safe. If the drain probe or drainer start-up throws, the partially
    initialized handle is destroyed and any started drainers are joined before rethrowing; the handle
    is published only once all drainers are running.
  • io_uring drain compatibility. Below kernel 5.11 (IORING_FEAT_EXT_ARG absent) liburing emulates
    a wait timeout by posting a timeout SQE, which mutates the submission queue from the completion
    side. The drainer detects the missing feature at ring init and polls the completion queue instead.

Key Technical Details

Affected types:

  • NativeStorageDevice — the managed P/Invoke wrapper; sharded in-flight tracking, slot pool,
    handle leasing, completion draining, smart defaults.
  • NativeDeviceOptions / LocalMemoryDeviceOptions (new, DeviceOptions.cs) — group the
    device-type-specific tuning that previously sprawled across positional parameters.
  • ConcurrencySharding (new) — the min(2 × ProcessorCount, cap) sizing formula, shared by the
    device in-flight shards and the buffer pool depot stripes.
  • BufferPool.OriginReturn.cs — depot stripe count derived from that formula (stripe count only).
  • file_linux.{h,cc}, native_device.h, native_device_wrapper.cc, thread.h — native submit /
    completion / thread-registration paths.

New server options (all default to today's behavior): --device-io-contexts,
--device-queue-depth, --device-uring-sqpoll, --device-uring-sqpoll-idle-ms,
--device-aio-max-devices. The pre-existing --device-throttle-limit gains a Native-device
default of 4096 (sized for deep NVMe / io_uring queues) rather than the managed devices' 120.

Docs: new Device Tuning guide
derives every knob and internal constant, including which combinations are actually reachable.

Breaking Change

Devices.CreateLogDevice, LocalStorageNamedDeviceFactory and
LocalStorageNamedDeviceFactoryCreator drop their loose ioBackend / localMemorySegmentSize /
localMemoryRingCapacity parameters in favour of the NativeDeviceOptions /
LocalMemoryDeviceOptions objects. This is a deliberate signature break agreed for this PR
(these entry points are not part of the supported public surface); callers pass an options object
instead.

What NOT to Do (for future agents)

  • Don't tie io_uring ring count to the completion-thread count. Rings must scale with
    submitter concurrency; too few rings serialize submitters on a per-ring lock and cost up to ~3×.
  • Don't add a global in-flight counter or a shared free-slot queue on the submit path. That
    is precisely the cache-line ping-pong this PR removed.
  • Don't let a managed exception escape the device completion callback. It crosses the C ABI
    boundary and silently terminates the drainer thread, leaving every later IO pending forever.
  • Don't raise --device-throttle-limit above 4096 expecting more in-flight IO. Device-wide
    in-flight is bounded by NumShards × MaxPerThreadInFlight; raising the ceiling means growing
    SlotsPerShard, not the throttle.
  • Don't rewrite an SQE after io_uring_submit on the SQPOLL path. The poll thread may have
    already consumed it; the non-SQPOLL no-op-rewrite unwind trick is not valid there.
  • Don't call io_uring_wait_cqe_timeout from the drainer without checking IORING_FEAT_EXT_ARG.
    On kernels that lack it, liburing emulates the timeout by taking an SQE and flushing the submission
    queue — from the completion side, without the submit lock, and with a non-null reserved user_data
    the batch dispatcher would treat as a caller context.
  • Don't compare device/KV benchmark numbers across runs on a busy box. Leftover build daemons
    on the pinned NUMA node depress results by ~30% and have already produced one phantom regression.
    Measure both arms back-to-back.

Testing

  • DeviceTests.cs (+117), most cases run against both libaio and io_uring: multi-drainer
    parallel mixed IO, multiple devices across threads with no cross-shard slot reuse, high-throttle
    high-in-flight, throttle-above-max clamping, high-concurrency many-threads no-hang, explicit
    io-context/ring-count with multi-ring fan-out submitted from concurrent threads, explicit shallow
    queue depth exercising ring-full backpressure, SQPOLL round-trip, and Reset / RemoveSegment /
    GetFileSize / TryComplete behavior.
  • GarnetServerConfigTests.cs (+101): default propagation and override parsing for every new option.
  • Benchmarks: Device.benchmark, KV.benchmark and Resp.benchmark READMEs carry the reproduction
    commands and current measured numbers; benchmark/Resp.benchmark/scripts/nvme-raid0-matrix.sh
    regenerates the RESP matrix.

Copilot AI left a comment

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.

Pull request overview

Optimizes Tsavorite’s native Linux I/O path for high-concurrency NVMe workloads through configurable ring topology, sharded bookkeeping, and reduced contention.

Changes:

  • Adds independent I/O-context, queue-depth, throttle, and SQPOLL tuning.
  • Shards buffer/completion state and introduces affine completion draining.
  • Adds configuration plumbing, benchmarks, tests, and tuning documentation.

Reviewed changes

Copilot reviewed 43 out of 55 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
website/sidebars.js Adds device-tuning guide.
website/docs/getting-started/configuration.md Documents new settings.
website/docs/dev/device-tuning.md Adds detailed tuning guide.
test/standalone/Garnet.test/GarnetServerConfigTests.cs Tests option plumbing.
test/cluster/Garnet.test.cluster/ClusterTestUtils.cs Adds endpoint failover wait.
test/cluster/Garnet.test.cluster/ClusterManagementTests.cs Avoids failover test race.
libs/storage/Tsavorite/cs/test/test.hlog/DeviceTests.cs Updates device-test commentary.
libs/storage/Tsavorite/cs/src/core/Utilities/BufferPool.cs Stripes buffer free lists.
libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/TsavoriteThread.cs Uses affine completion draining.
libs/storage/Tsavorite/cs/src/core/Index/CheckpointManagement/LocalStorageNamedDeviceFactoryCreator.cs Plumbs device options.
libs/storage/Tsavorite/cs/src/core/Index/CheckpointManagement/LocalStorageNamedDeviceFactory.cs Applies device options.
libs/storage/Tsavorite/cs/src/core/Device/StorageDeviceBase.cs Extends completion API.
libs/storage/Tsavorite/cs/src/core/Device/NativeStorageDevice.cs Implements core I/O optimizations.
libs/storage/Tsavorite/cs/src/core/Device/LocalStorageDevice.cs Adapts completion API.
libs/storage/Tsavorite/cs/src/core/Device/IDevice.cs Adds affine-drain parameter.
libs/storage/Tsavorite/cs/src/core/Device/Devices.cs Exposes native tuning options.
libs/storage/Tsavorite/cs/src/core/Allocator/AllocatorBase.cs Selects affine/all-ring draining.
libs/storage/Tsavorite/cs/benchmark/KV.benchmark/README.md Updates NVMe guidance.
libs/storage/Tsavorite/cs/benchmark/KV.benchmark/Options.cs Adds benchmark options.
libs/storage/Tsavorite/cs/benchmark/KV.benchmark/KvOutput.Json.cs Reports renamed throttle option.
libs/storage/Tsavorite/cs/benchmark/KV.benchmark/KvOutput.Csv.cs Reports renamed throttle option.
libs/storage/Tsavorite/cs/benchmark/KV.benchmark/KvBenchmark.Setup.cs Applies tuning options.
libs/storage/Tsavorite/cs/benchmark/Device.benchmark/README.md Documents RAID-0 results.
libs/storage/Tsavorite/cs/benchmark/Device.benchmark/Program.cs Configures new device knobs.
libs/storage/Tsavorite/cs/benchmark/Device.benchmark/Options.cs Adds benchmark flags.
libs/storage/Tsavorite/cs/benchmark/Device.benchmark/BenchWorker.cs Reduces completion polling.
libs/storage/Tsavorite/cc/src/device/thread.h Pads thread-ID flags.
libs/storage/Tsavorite/cc/src/device/thread_manual.cc Defines padded flags.
libs/storage/Tsavorite/cc/src/device/native_device.h Extends native device interface.
libs/storage/Tsavorite/cc/src/device/native_device_wrapper.cc Extends native C ABI.
libs/storage/Tsavorite/cc/src/device/file_windows.h Adapts Windows handler.
libs/storage/Tsavorite/cc/src/device/file_linux.h Adds ring and SQPOLL support.
libs/storage/Tsavorite/cc/src/device/file_linux.cc Implements batched affine draining.
libs/server/Servers/GarnetServerOptions.cs Plumbs server settings.
libs/host/defaults.conf Defines new defaults.
libs/host/Configuration/Options.cs Adds CLI/config options.
benchmark/Resp.benchmark/scripts/nvme-raid0-matrix.sh Adds RAID-0 benchmark matrix.
benchmark/Resp.benchmark/README.md Documents RESP results.
benchmark/Resp.benchmark/Program.cs Applies load-thread setting.
benchmark/Resp.benchmark/Options.cs Adds load-thread option.
benchmark/Resp.benchmark/OfflineBench/RespPerfBench.cs Removes request allocations.
benchmark/Resp.benchmark/OfflineBench/ReqGen.cs Caches complete argument arrays.
.github/workflows/native-build.yml Verifies native exports.
Suppressed comments (2)

benchmark/Resp.benchmark/scripts/nvme-raid0-matrix.sh:111

  • The GET client also ignores the configurable PORT, so benchmark passes target port 6379 rather than the server started above when PORT is overridden. Forward --port "$PORT" here too.
      local out; out="$($cli_pin dotnet "$RB" -s --op GET --dbsize "$DBSIZE" \
        --keylength "$KEYLEN" --valuelength "$VALLEN" --client LightClient \
        -t "$t" -b "$REQB" --runtime "$RUNTIME" 2>/dev/null)"

website/docs/dev/device-tuning.md:272

  • The constants table does not match the implementation: NumShards is min(2 × ProcessorCount, 32), not a 128–1024 clamp, and ReconcileIntervalMs does not exist. These values materially change the documented memory footprint and contention model.

Comment thread libs/storage/Tsavorite/cc/src/device/native_device_wrapper.cc
Comment thread libs/storage/Tsavorite/cc/src/device/native_device_wrapper.cc Outdated
Comment thread libs/storage/Tsavorite/cs/src/core/Device/IDevice.cs Outdated
Comment thread libs/storage/Tsavorite/cs/benchmark/KV.benchmark/Options.cs Outdated
Comment thread libs/storage/Tsavorite/cs/benchmark/Device.benchmark/Options.cs Outdated
Comment thread libs/storage/Tsavorite/cs/benchmark/KV.benchmark/Options.cs Outdated
Comment thread libs/storage/Tsavorite/cs/benchmark/KV.benchmark/README.md Outdated
Comment thread libs/storage/Tsavorite/cs/benchmark/KV.benchmark/README.md Outdated
@badrishc
Badrish Chandramouli (badrishc) marked this pull request as ready for review August 7, 2026 23:46
Badrish Chandramouli (badrishc) added a commit that referenced this pull request Aug 9, 2026
…uffer-pool mode

Adds an opt-in native (off-managed-heap) allocator, first surface: the
SectorAlignedBufferPool IO buffers. Its single per-level ConcurrentQueue is a
measured cache-line contention bottleneck under concurrent rent/return (the same
knee PR #2018 shards); mimalloc's thread-local heaps + cross-thread free lists
replace the manual recycling and also subsume the cross-thread-return handling.

Foundation (libs/.../core/Native/):
- Mimalloc.cs: explicit NativeLibrary.Load + GetExport function-pointer binding
  (Tsavorite.core already owns the single per-assembly DllImportResolver, so we
  do not add a second). Ships prebuilt libmimalloc.so under
  Native/runtimes/<rid>/native/ via the csproj, loaded on demand; absence falls
  back to managed.
- INativePinnedAllocator + MimallocPooledAllocator (mi_malloc_aligned /
  mi_zalloc_aligned / mi_free); NativeMemoryTracker (telemetry only — no
  GC.AddMemoryPressure); NativeAllocatorSurfaces flags; NativeAllocatorInitializer
  public startup entry point.

SectorAlignedBufferPool:
- When SectorAlignedBufferPool.NativeAllocator is set (buffer-pool/full mode), Get
  bypasses the queue: clearOnReturn:false -> mi_malloc (device-read dest, no
  memset), clearOnReturn:true -> mi_zalloc (zero tail); Return -> mi_free. Wrapper
  objects are recycled thread-locally to avoid Gen0 churn. Managed path unchanged.

Host wiring:
- --native-allocator off|buffer-pool|full (+ --native-allocator-require), mapped
  to GarnetServerOptions.NativeAllocatorSurfaces and installed in
  GarnetServer.InitializeServer before any store/pool is created. defaults.conf
  updated. (full-mode direct-VM singletons scaffolded; wired in a later phase.)

Tests: NativeAllocatorTests (load, round-trip, alignment, zeroing, cross-thread
return, wrapper recycling, tracker). NativeAllocatorPerfTests ([Explicit]) A/B
shows managed pool collapsing to ~1 Mops/s under contention while mimalloc holds
~8-9 Mops/s (5-10x at 4-32 threads). Managed regression (BasicLockTests) green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a49f1db0-fd39-48c7-80dc-ce104c863b79
Badrish Chandramouli (badrishc) added a commit that referenced this pull request Aug 9, 2026
…lt check

Adds [Explicit] profiling to explain the single-thread managed>mimalloc gap and
verify the experiments are measuring what we think.

Findings:
- Page faults: over 5M rent/return ops, minflt/majflt are ~0 for BOTH managed and
  mimalloc (~0.0001 faults/op). No physical memory is faulted per-op; both reuse a
  hot committed block. So the ns/op numbers are pure allocator bookkeeping, not
  allocation cost.
- SuppressGCTransition variants (mi_*alloc/mi_free): ~0ns benefit on .NET 10 — the
  function-pointer GC transition is already sub-ns. Kept only as profiling evidence.
- Single-thread bookkeeping breakdown (ns/op): managed reuse ~30; raw mi_malloc
  ~17, mi_malloc_aligned ~24; via pool ~72 (+48 pool/interface/wrapper), tracker
  +15. Alignment slow-path ~5ns (plain mi_malloc(4096) is already 512/4096-aligned,
  but we keep mi_malloc_aligned for the guarantee).
- Real-world dilution: writing the full 4KB buffer (like real IO) shrinks the
  single-thread gap from 3.0x to 1.7x; zeroing (clr=true) adds +39ns managed /
  +20ns mimalloc — confirming the clearOnReturn:false no-memset path matters.

Conclusion: the single-thread deficit is bookkeeping-only and dilutes to near-noise
once buffers are used; the real number needs the KV/Device/RESP e2e A/B vs the
PR #2018 sharded pool. The ~15ns per-op tracker is a separately-fixable overhead
(query mi_process_info on demand instead of per-op accounting).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a49f1db0-fd39-48c7-80dc-ce104c863b79
Badrish Chandramouli (badrishc) added a commit that referenced this pull request Aug 9, 2026
Adds a --native-buffer-pool flag to KV.benchmark and Device.benchmark that installs
the mimalloc-backed SectorAlignedBufferPool (buffer-pool mode) at startup, for
managed-vs-native A/B.

Note: Device.benchmark's timed worker uses its own per-worker NativeMemory buffer,
so only its fill/verify phase touches the pool; KV.benchmark (real TsavoriteKV store)
is the meaningful pool A/B via AllocatorBase.bufferPool on the disk-read staging path.

End-to-end KV.benchmark result (LocalMemory device, 1M keys, 100% reads, zipf, 32 threads):
- disk-read-heavy (log 32MB << 128MB dataset): managed 2.82 M ops/s vs native 25.65 M
  ops/s = 9.1x. Managed DEGRADES 8->32 threads (contention); native SCALES.
- in-memory control (log 256MB >= dataset): 72.4 M vs 86.1 M ops/s = 1.19x.
The 9.1x-vs-1.19x contrast isolates the disk-read bufferPool path as the cause (not a
general mimalloc effect). Baseline is the UNSHARDED pool; PR #2018 sharding would
narrow the managed gap, but mimalloc reaches it without hand-rolled striping. gc=0 in
all runs. See session kv-benchmark-ab.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a49f1db0-fd39-48c7-80dc-ce104c863b79

@TedHartMS Ted Hart (TedHartMS) left a comment

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.

Approved with nit comments

Comment thread libs/host/defaults.conf Outdated
Comment thread libs/host/Configuration/Options.cs Outdated
Comment thread libs/storage/Tsavorite/cs/src/core/Device/Devices.cs Outdated

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 43 out of 55 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

libs/storage/Tsavorite/cs/src/core/Device/NativeStorageDevice.cs:959

  • PerThreadLimit makes every configured throttle above 4096 ineffective: activeShards is at most 32 and each shard is capped at 128, so both --device-throttle-limit 4096 and 65536 produce the same per-shard limits for every possible active-shard count. This contradicts the new 65536 tuning guidance and prevents the advertised higher aggregate in-flight depth. The shard/slot sizing or cap needs to scale with the effective throttle.
    libs/storage/Tsavorite/cs/src/core/Device/Devices.cs:36
  • This public method removes the existing ioBackend, localMemorySegmentSize, and localMemoryRingCapacity parameters and changes the type/order immediately after readOnly. Existing named calls no longer compile, positional calls can bind incorrectly, and already-compiled consumers cannot resolve the old CLR signature. Preserve the original overload and introduce the options-based API through a non-ambiguous overload or new method.
        public static IDevice CreateLogDevice(string logPath = null, DeviceType deviceType = DeviceType.Default, bool preallocateFile = false, bool deleteOnClose = false, long capacity = CAPACITY_UNSPECIFIED, bool recoverDevice = false, bool useIoCompletionPort = false, bool disableFileBuffering = true, bool readOnly = false, int numCompletionThreads = 1, ILogger logger = null, NativeDeviceOptions nativeDeviceOptions = null, LocalMemoryDeviceOptions localMemoryDeviceOptions = null)

libs/storage/Tsavorite/cs/src/core/Index/CheckpointManagement/LocalStorageNamedDeviceFactory.cs:42

  • Removing the public constructor's ioBackend parameter changes the established signature; old positional calls now pass an IoBackend where int numCompletionThreads is expected, and named ioBackend: calls fail. This also reintroduces the compatibility problem previously addressed for this constructor. Keep the old constructor as a forwarding overload while exposing NativeDeviceOptions separately.
    libs/storage/Tsavorite/cs/src/core/Index/CheckpointManagement/LocalStorageNamedDeviceFactoryCreator.cs:35
  • This public constructor likewise removes ioBackend, breaking existing positional and named callers even though the new options object is appended. Retain the previous signature as a compatibility overload that constructs NativeDeviceOptions, rather than replacing it outright.
    website/docs/dev/device-tuning.md:354
  • The configuration page is in docs/getting-started, not docs/dev, so this relative link resolves to a nonexistent dev/configuration page. Point it to the existing sibling-directory path.
    benchmark/Resp.benchmark/scripts/nvme-raid0-matrix.sh:103
  • The load command's exit status is ignored because the script does not use set -e or check this invocation. If loading fails, the script still benchmarks GET misses and can publish a high but meaningless “NVMe” throughput table. Abort the configuration when the load command fails (and ideally verify the loaded key count) before entering the measured sweep.
  $cli_pin dotnet "$RB" --port "$PORT" --op MSET --dbsize "$DBSIZE" --keylength "$KEYLEN" --valuelength "$VALLEN" \
    --client LightClient --load-threads 32 -b 4096 --runtime 0 >/tmp/nvme-matrix-load.log 2>&1

libs/storage/Tsavorite/cs/src/core/Utilities/ConcurrencySharding.cs:18

  • The stated invariant is impossible on hosts with more than 16 logical processors: the formula caps the count at 32, so it cannot remain at or above 2 × ProcessorCount. Reword this to state that the count scales at two per processor only on small hosts and deliberately allows shard sharing after the cap.
    libs/storage/Tsavorite/cs/test/test.hlog/DeviceTests.cs:1367
  • This test does not exercise the claimed multi-ring fan-out: all ReadAsync calls run on the single NUnit thread, while pick_context_index() assigns one affine ring per submitting thread. Consequently all 128 reads target one of the eight rings, so regressions in cross-thread ring assignment or draining the other seven rings remain undetected. Submit from at least eight concurrent threads and verify all completions.

Comment thread libs/storage/Tsavorite/cc/src/device/file_linux.cc
…VMe RAID-0)

WIP: raise Garnet SSD random-read serving throughput toward the fio ceiling
(8.24M IOPS). Verified on KV.benchmark scenario 2 (100M x 100B, log-mem 16m,
100% random 4K reads from disk, native libaio), standalone runs, node0-pinned.

Fixes in this branch:
- FIX#1 (NativeStorageDevice.cs): shard per-submitter in-flight tracking; removed
  the global numPending counter + freeResults queue that cache-line ping-ponged
  across all submit/complete threads. Positive thread scaling.
- FIX#2 (Device.benchmark/BenchWorker.cs): drop the per-op device.TryComplete()
  from the submit hot path (serialized all submitters on ctx0's kernel ring_lock);
  dedicated drainers do all reaping.
- FIX#3 (cc/device/thread.h, thread_manual.cc): cache-line-pad Thread::id_used_[]
  (per-IO EpochGuard acquire/release CAS was false-sharing). Native .so rebuilt.
- FIX#4 (NativeStorageDevice.cs): per-shard free-list for completion slots. The
  prior counter-ring slot reuse was unsafe under OUT-OF-ORDER device completion
  (a slow IO's slot could be overwritten by newer submits wrapping the ring),
  corrupting the AsyncIOContext and crashing KV. Slots now return to a free-list
  only after their own IO completes.
- FIX#5 (Utilities/BufferPool.cs): stripe SectorAlignedBufferPool's per-level
  free-list across 128 sub-queues, thread-affine. The single ConcurrentQueue per
  size-level was 52.6% of all CPU (TryDequeue+TryEnqueue) under the pending-read
  workload. 4.14M -> 6.30M @32thr. General win for all Garnet disk reads.
- FIX#6 (NativeStorageDevice.cs, Devices.cs, benchmarks): decouple num_io_contexts
  from num drainer threads via new numIoContexts option (--device-io-contexts /
  --io-contexts). Default 0 == legacy 1:1 (byte-for-byte unchanged). Multi-ring
  drainers range-POLL their rings (never block on one ring, which would starve the
  siblings). Helps low/medium thread counts; neutral at peak.

Peak verified: 6.94M ops/s @48 threads (84% of fio). All 89 DeviceTests pass.

TODO (not in this commit): batched io_submit; io_uring IOPOLL + registered
buffers/files; patchelf .so libaio.so.1t64 -> libaio.so.1 + libaio-only variant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e drain

Two additional, opt-in (default = legacy behavior) levers explored while closing
the KV random-read gap on 8x NVMe RAID-0. Both were measured neutral at the
core-saturated peak (~6.9M) but positive at lower/medium thread counts, and are
kept behind env flags with zero-regression defaults.

FIX#7 batched libaio submit (GARNET_SUBMIT_BATCH, default 1 = immediate submit):
- Native (file_linux.cc/.h, native_device.h, native_device_wrapper.cc): per-submitter
  thread_local accumulation of prepared READ iocbs, flushed via io_submit(ctx, N) at a
  threshold; handles partial submit / EAGAIN / per-iocb permanent error. New
  NativeDevice_FlushSubmits C ABI (uring backend = no-op).
- Managed (NativeStorageDevice.cs, IDevice.cs, StorageDeviceBase.cs): FlushSubmits() +
  P/Invoke; TryComplete() flushes the calling thread's batch first (throttle-spin safety
  net). KV benchmark (KvBenchmark.Worker.cs) flushes the sub-threshold tail per read batch.

Affine inline drain (GARNET_INLINE_DRAIN_AFFINE, default off):
- New IDevice.TryCompleteMine() (StorageDeviceBase falls back to TryComplete; native
  NativeStorageDevice drains only the caller's affine context/ring). The inline
  submitter-thread completion path (TsavoriteThread.cs, AllocatorBase.cs) uses it when
  the flag is set, cutting per-context io_getevents syscalls at lower thread counts.

Rebuilt both prebuilt Linux natives from current source (USE_URING=ON ->
libnative_device.so, USE_URING=OFF -> libnative_device_libaio.so) so both export the
new NativeDevice_FlushSubmits / NativeDevice_TryCompleteMine entrypoints; TryComplete
calls FlushSubmits unconditionally, so the libaio-only fallback must have them too.

Tsavorite build clean (0 warnings); 89 DeviceTests pass; default path is
byte-for-byte legacy behavior (both env flags unset).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two concurrency bugs made the offline benchmark crash (NullReferenceException
in ReqGen.GetRequestArgs) and produce malformed requests under load:

1. Shared request list mutated in place. GetRequestArgs() returns a reference
   to a cached List<string> owned by ReqGen. GarnetClientSessionOperateThreadRunner
   did reqArgs.Insert(0, "MSET") on it every iteration. That both (a) raced across
   threads (concurrent List.Insert corrupts the backing array) and (b) permanently
   prepended "MSET" to the shared list, growing it unboundedly even single-threaded.
   Fix: build a fresh args array with the command prepended; never mutate the cache.

2. Non-thread-safe System.Random for serve-offset selection. GetRequest() and
   GetRequestArgs() called a shared System.Random ('r') concurrently from all
   worker threads, which can return out-of-range indices. Fix: use Random.Shared
   (thread-safe) for the concurrent serve-offset draw. 'r' is retained for the
   single-threaded generation phase where its deterministic seed matters.

Also add --load-threads (default 8) to parallelize the initial data-load phase.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… degradation)

The inline submitter-thread completion drain (Tsavorite CompletePending /
AsyncGetFromDisk throttle-wait) is the primary reaper for disk-bound reads.
The legacy inline drain, IDevice.TryComplete(), reaps only a single fixed
io_context (context 0), so every inline-draining thread serialized on that
one context's kernel aio mutex.

On the RESP GarnetServer, disk-read completions are processed by the .NET
thread pool, which grows under disk latency. With many workers all draining
context 0, ~46% of server CPU was spent spinning on the aio mutex (osq_lock),
and 100% random-read throughput spiralled downward run-over-run (6.75M -> 2.4M
ops/s on an 8x NVMe RAID-0) as the pool grew. The fixed-context drain also
only covered context 0's 1/N share of completions inline.

IDevice.TryCompleteMine() reaps the calling thread's own affine context (the
one its submits land on), spreading the inline drain across all contexts. This
removes the mutex storm (osq_lock 46% -> 7%) and holds throughput stable at
~7.2M ops/s across many runs with no degradation. It was previously measured
neutral at the uncontended saturated peak, so making it the default is a strict
improvement. Devices that do not shard completions fall back to TryComplete()
automatically, so this is safe for all device types.

Flip Constants.InlineDrainAffine to default on; set GARNET_INLINE_DRAIN_AFFINE=0
to restore the legacy fixed-context-0 drain. 89 DeviceTests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…libaio parity)

Defer io_uring_submit to coalesce many read SQEs into one submit syscall,
mirroring the existing libaio batch. Opt-in via GARNET_SUBMIT_BATCH (default
1 = submit per-op = byte-for-byte legacy, zero regression). Only a thread that
solely owns its ring defers (per-ring CAS ownership); ring-sharing threads
(submitters > rings) fall back to per-op submit. Writes never batched.

Every deferred SQE carries its io_context as user_data, so exactly one
completion is dispatched regardless of which thread flushes. The managed
TryComplete/TryCompleteMine (and the AsyncGetFromDisk throttle-spin) already
call NativeDevice_FlushSubmits before draining, upholding the flush-before-wait
invariant (no throttle deadlock) with no managed changes. get_sqe==null flushes
the pending batch before retry/unwind; a real UringIoHandler::FlushSubmits with
-EBUSY drain-assist (TryCompleteFor) replaces the previous no-op stub.

Device.benchmark (512B random reads, t=32, io-contexts=32, no pin): uring
7.85M -> 8.94M at batch=32 (+14%), a new uring high reaching ~parity with
libaio-batched (9.23M); integrity verified (71.5M ok == submitted, 0 err).
Confirms per-op submit was uring's disadvantage vs libaio's batched io_submit.
RESP serving is unchanged (closed-loop, managed-CPU-bound; batching neutral).

BenchWorker.cs: flush the deferred tail batch via TryComplete in the shutdown
drain so a sub-threshold tail never strands the exit. Native .so rebuilt
(uring + libaio variants; libaio path is functionally unchanged).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…m drainers

Give each submitter thread its own ring (rings >= completion threads) to avoid
the shared-ring non-owner submit path, independently of the drainer count. The
value is clamped up to --device-completion-threads so every drainer owns at
least one ring.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the device tuning flags (--device-io-contexts, --device-completion-threads,
--device-throttle-limit, --device-io-backend) and the record/page/segment settings
used to reproduce the current SSD random-read numbers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The inline submitter-thread completion path (Tsavorite CompletePending /
AsyncGetFromDisk throttle-wait) drains the caller's own affine ring via
TryCompleteMine, which reaped exactly one CQE per call (TryCompleteFor ->
io_uring_peek_cqe). The dedicated drainer's QueueRunFor already batch-reaps up
to kCqeBatch CQEs per cq_lock section, and libaio's QueueIoHandler::TryCompleteMine
already batches via io_getevents; the uring inline path did not, so the network
thread paid one cq_lock + one dispatch loop iteration per completion on the hot
critical path.

Add UringIoHandler::TryCompleteMineBatch: a non-blocking single pass that reaps
up to kCqeBatch (64) completions in one cq_lock section (io_uring_peek_batch_cqe
-> snapshot -> io_uring_cq_advance -> release -> dispatch outside the lock),
mirroring QueueRunFor's phase-2. TryCompleteMine now calls it. One managed
NativeDevice_TryCompleteMine P/Invoke therefore delivers a batch of completions,
which the same thread drains inline.

On disk-served RESP GET (100M x 128B, uring, 96 rings, 2 drainers) this raises
throughput about 20% at the t=48 peak (5.94M -> ~7.1M ops/s) and removes the
prior drainer-count sensitivity (the network threads now self-drain in batches,
so a single background drainer no longer collapses under throttle-spin). The
libaio backend is unaffected (its TryCompleteMine already batches); the
UringIoHandler code is compiled only under FASTER_URING, so the libaio-only
prebuilt is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…h-reap

Make the io_uring completion batch-reap (TryCompleteMineBatch) an opt-out knob so
its throughput impact can be measured on the same binary without a revert-build.
Default ON (unchanged behavior); GARNET_URING_BATCH_REAP=0 falls back to the legacy
single-CQE reap (TryCompleteFor). Read once via a function-local static.

Consistent with the other env-gated device levers (GARNET_DEVICE_IO_CONTEXTS,
GARNET_SUBMIT_BATCH). Same-build ablation shows batch-reap is neutral at the
saturated peak (a harmless CPU efficiency, not a peak-throughput lever), so this
gate documents that finding and keeps it toggleable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…device queue depth

The per-submitter-thread in-flight sharding split the global ThrottleLimit
into a per-thread budget via PerThreadLimit() = ThrottleLimit / activeShards.
activeShards was Interlocked.Increment-ed the first time each thread submitted
(AssignShard) but never decremented. Under the .NET ThreadPool, submitter
threads are transient: they retire when idle and fresh ones are injected on the
next burst. Each fresh thread bumped activeShards, so over a long-lived device
the divisor ratcheted up without bound even though the number of concurrently
active submitters stayed roughly constant.

As a result the per-thread in-flight budget collapsed over the process's
lifetime (e.g. 4096/35 -> 4096/245), so the aggregate device queue depth
starved from ~ThrottleLimit down to a small fraction of it, and disk-serving
read throughput declined progressively (observed ~55% over successive RESP GET
runs), recoverable only by restarting the server. In-memory workloads were
unaffected because they never exercise the pending-read throttle path.

Fix: keep AssignShard's immediate increment (new threads instantly get a fair
share) but periodically reconcile activeShards DOWN to actual shard occupancy
(shards with in-flight > 0, ~= concurrently active submitters) via
MaybeReconcileActiveShards(), time-gated to at most once per 200 ms by a cheap
non-atomic tick check on the completion path. Births are counted immediately;
deaths are reclaimed lazily from live occupancy. This only resizes the throttle
divisor (a perf knob) and does not touch slot allocation, the submitted/completed
balance, or completion routing, so it cannot affect correctness.

Validated: RESP disk GET is now flat across successive idle-gap-separated runs
(was a monotonic ~55% collapse); 89 DeviceTests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…FINITY)

The committed uring model owns a ring per submitter thread via a sticky
`ring_owner_[idx]` that is released only at teardown. On .NET's oversubscribed
ThreadPool this orphans rings: a retired thread leaves its tid in the owner
slot forever, so that ring can never batch-own again and degrades to per-op
submit ("dead-tid poisoning").

This adds an opt-in ownership model that maps LightEpoch's thread-affinity
pattern onto ring ownership, env-gated by GARNET_RING_LE_AFFINITY (default off
= byte-identical legacy path):

  - thread-local preferred ring (== LightEpoch startOffset1), re-acquired warm
    each network batch;
  - probe-and-replace on collision (== TryAcquireEntry circling the table):
    CAS-claim a free ring and adopt it as the new preferred;
  - release at the network-batch boundary (== Release-on-Suspend): the reliable
    "suspend" point after which the thread will not submit again until its next
    batch, so a churned-away thread's ring is reclaimed instead of orphaned.

The batch-boundary release is gated on actual disk-read activity: a
`[ThreadStatic]` flag is set in NativeStorageDevice.ReadAsync (the single choke
point every disk read funnels through; pure in-memory hits never reach it) and
consumed by EndBatchReleaseRing() in RespServerSession's batch finally. A batch
that touched no disk pays one thread-static check and no P/Invoke, so the
feature is free on in-memory and mixed workloads.

Native: file_linux.{h,cc} add le_affinity_enabled() (cached), pick_ring_index_le()
(warm fast-path / CAS-claim / probe-adopt), release_my_ring(), and a unified
uring_thread_id() so pick and submit agree on the owner id; libaio's
QueueIoHandler gets a no-op release_my_ring(). native_device.{h} +
native_device_wrapper.cc export NativeDevice_ReleaseRing. AllocatorBase /
LogAccessor expose the log IDevice so the session can reach the device.

Measured (pinned, uring, 96 rings, submit-batch 32, fresh 100M keylen16 val96,
OFF/ON/OFF bracket): throughput-neutral vs the committed model on both 100%
random disk read (t=32 ~6.99M, t=48 ~7.9M; ON bracketed by OFF) and in-memory
(t=32 ~52M, t=48 ~85M). 89 DeviceTests pass; GarnetServer builds warning-free.
RESP disk serving is managed-CPU-bound, so this is a cleaner, non-poisoning
ownership model rather than a throughput lever; kept opt-in pending a broader
workload matrix (libaio, mixed, churn soak).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The branch's native device changes added three methods that NativeDeviceImpl<H>
forwards to its handler_ (TryCompleteMine, FlushSubmits, and release_my_ring via
ReleaseRing). These were only implemented on the Linux libaio/io_uring handlers.
The Windows default (NativeDeviceImpl<ThreadPoolIoHandler>) failed to compile under
MSVC once the native-build workflow exercised the Windows RIDs:

  error C2039: 'release_my_ring' is not a member of 'ThreadPoolIoHandler'

Add the three as no-ops on ThreadPoolIoHandler, matching the existing Windows IOCP
no-op style: completions fire on threadpool threads (no caller-affine inline drain),
submits are immediate (no batch to flush), and io_uring ring ownership is Linux-only
(nothing to release). Linux-only file, no effect on the Linux build.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerated by the Build Native Device workflow (run 30764320573) from the native sources on 'badrishc/optimize-device-iops'.
…k sizes

The Device/KV/Resp benchmark READMEs compared Garnet throughput measured with
512 B sector reads against a fio ceiling quoted only at 4 K, without saying the
two block sizes are comparable. The array is IOPS-bound rather than
bandwidth-bound in this range, so the same fio job yields 8.24 M IOPS at 4 K and
8.20 M IOPS at 512 B; the parity percentages are unchanged, but the READMEs now
quote both figures so the comparison is explicit.

Also drops the inaccurate "4 KB-class random reads" description of the RESP
workload, which reads 128 B records over the array's 512 B sectors.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The load-verification guard read a fixed 32 bytes from the raw RESP socket, but
the ":<n>\r\n" integer reply is shorter than that, so on a host without redis-cli
the read blocked forever and the matrix never advanced past its first load.

Read a single terminated line with a timeout instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DBSIZE scans the whole hash index, so on the 100 M-key store it does not reply
within the probe window and the guard read back an empty count. The loader
already reports the number of ops it pushed, which is the same signal without
a server round trip. Route both logs through overridable variables.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Disk-serving RESP GET ran ~31% below the same workload on the pre-rebase tree.
A cross-commit A/B (two order-flipped rounds, three passes each, fresh 100M-key
load per arm) put the regression at 6.72 M ops/s versus 4.62 M with fully
disjoint ranges, and a perf call graph placed ~11.6% of server CPU in
unresolved libcoreclr frames under NetworkGET_SG that were absent from the
faster arm.

Instrumenting the pool located it. Buffer gets split 25% owner-local / 75%
shared depot, and the depot's stripes are locked stacks, so ~186 threads drove
roughly 8 M Monitor.Enter per second across only 8 locks. The stripe count was
fixed at 8 regardless of the hardware, so the number of threads that can enter
the depot concurrently did not scale with the machine: sized for a small box it
serializes a large one. It now derives from ConcurrencySharding, which already
sizes the device in-flight shards this way, rounded up to a power of two so the
existing mask indexing still applies and floored at 8 so small boxes keep their
current width.

The cap is 64, which covers the common concurrency range. A stripe sweep at 48
client threads (three order-rotated rounds of three passes, fresh 100M-key load
per run) gives 4.51 / 5.15 / 5.73 / 6.85 / 6.82 M ops/s at 8 / 16 / 32 / 64 /
128 stripes, so throughput is flat from 64 onward there. Because a server drives
the depot from roughly one thread per connection, the knee does move out at much
higher connection counts: at 128 client threads (~266 server threads) 64 stripes
gives 5.85 M against 6.70 M at 256. That degradation is accepted in exchange for
the smaller stripe array and shorter miss scan; workloads that sustain far more
concurrent threads than the cap trade some throughput for those bounds.

Widening does not strand buffers, because a depot miss already scans every
stripe of the size class rather than only the caller's; the extra stripes cost
about 113 KB per pool and nothing per operation. No other pool constant changes,
so the byte budget still bounds retained memory exactly as before.

Raising LocalCap alongside this was measured and dropped: at the wider stripe
count, 128 / 256 / 512 land at 6.86 / 6.96 / 7.02 M ops/s over three
order-rotated rounds of three passes, so the larger caps buy 1.4-2.3% while
raising the per-thread hoard bound, which nothing steals from until the thread
dies. The stripe count alone restores throughput.

The full NVMe matrix on device defaults peaks at 7.36 M ops/s (uring, pinned,
t=48), reproducing the published table within 4% on every pinned cell, with the
pool's fresh-allocation rate falling to zero.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove narration of prior behaviour, rejected alternatives, and measurement
history from the device tuning page, the benchmark README, and the device and
buffer pool comments, and cut the restatement that followed the floor / cap /
ceiling / headroom definitions. Each remaining statement describes what the code
does now and the constraint that shapes it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…modes

End-to-end review of the device changes (agent plus two independent model
reviews) surfaced five real defects. Each is fixed here; measurements below
confirm no throughput cost.

Use-after-free on the submit path. ReadAsync/WriteAsync bump their shard's
in-flight count before the P/Invoke, but that bump is dropped by the completion
callback running on a drainer thread. A fast completion can therefore drive the
count to zero while the submitting thread is still inside native code -- still
to run ~EpochGuard, which touches the device's epoch. Dispose can then observe
zero in-flight, join the drainers and destroy the device under the returning
submitter. Both entry points now take a second, independently balanced lease
around the native call, the same guard the non-IO entry points already get from
TryLease.

io_uring drainer raced SQ submission on kernels before 5.11. Without
IORING_FEAT_EXT_ARG, liburing emulates io_uring_wait_cqe_timeout by taking an
SQE, writing a timeout request with a reserved user_data and flushing the SQ --
from the completion side, without sq_lock, while submitters mutate the same SQ.
The reserved user_data is also non-null, so the batch dispatcher would have
treated it as a caller context. The feature bit is now sampled at init and the
drainer polls the completion queue instead when it is absent.

SQPOLL wakeup could be dropped. Once io_uring_submit publishes an SQE to an
SQPOLL ring the kernel owns it, so a failed enter means only that the wakeup was
not delivered to a parked poll thread; with no later submit on that ring the IO
never completes. The retry now backs off through sched_yield into bounded 1 ms
sleeps rather than giving up after a few yields.

Thread-start failure leaked the native device. The drainer slot was published
before Start(), so a failure under resource pressure left an unstarted thread in
the array; the cleanup path's Join() then threw ThreadStateException, escaped,
and skipped the destroy. The slot is now published after the thread is running
and the destroy runs in an unconditional finally.

A transient startup probe error published a device with no drainers.
NativeDevice_QueueRun doubles as the capability probe: Windows IOCP returns a
permanent negative, but a Linux backend can return a transient negative when the
probing thread is interrupted by a signal, which the runtime does routinely.
It is now retried before concluding the backend has no drainable queue.

Also: clamp RoundUpPow2 at IORING_MAX_ENTRIES so an out-of-range depth through
the C ABI cannot overflow; guard the completion callback's own logger call so a
throwing host logger cannot defeat the drainer firewall; and warn when a libaio
reservation cannot be brought within its per-device share of fs.aio-max-nr,
since depth cannot fall below one event per ring. The AioMaxDevices help text
and docs no longer claim an unconditional guarantee.

Device.benchmark, libaio, 512 B random reads on 8x NVMe RAID-0, 32 threads,
6 interleaved samples per arm: 8.469 M ops/s with the fixes against 8.527 M
before them, with fully overlapping ranges. The two extra interlocked operations
land on a shard line the submitter already owns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The `--device-aio-max-devices` section claimed the per-device reservation cap
guarantees at least that many devices can be created "regardless of the other
knobs". The cap cannot go below one event per ring, so a device configured with
more rings than its per-device share still exceeds it, and the budget is the
machine total rather than what remains after other processes. The derivation
section and the option help text already state both limits; this makes the knob
section agree and links to the derivation.

Also record the second reservation warning in the many-devices recipe: one fires
when `N x D` exceeds `fs.aio-max-nr`, the other when a device cannot be brought
within its per-device share.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The floor glossary entry claimed the reservation depth is never sized below
128, but the per-device AIO-budget clamp runs after the floor and halves past
it when the whole-device reservation does not fit. On a stock 65536 budget,
32 io-contexts resolve to a depth of 64.

State the precedence in both the doc and the constant's XML doc: the budget
ceiling overrides the floor, because exceeding the budget fails device creation
while a shallow ring only costs throughput.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rottle

The libaio reservation notes claimed multi-ring serving devices keep the full
aggregate throttle at no IOPS cost. That holds for the throttle-share math, but
the per-device fs.aio-max-nr ceiling runs last and caps effectiveThrottleLimit at
ringCount * depth. On a stock 65536 budget that bound is 2048, halving the default
4096 throttle at every ring count.

Qualify the claim in the doc and in the three matching code comments, and give the
operator the sizing rule: fs.aio-max-nr / --device-aio-max-devices >= throttle.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Throttle() gates a shard's whole in-flight against a limit clamped to
MaxPerThreadInFlight, so a shard holds at most that many slots however
many submitter threads share it. The SlotsPerShard summary attributed
the bound to a single submitter instead, which reads as if the free-list
could be drained by several threads sharing one shard. Match the framing
already used by Throttle() and RentSlot().

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The consequences list stated the no-IOPS-cost property unconditionally
and retracted it three bullets later, so a reader on a stock budget --
where the ceiling always binds -- takes away the wrong default. Attach
the property to the share clamps that provide it and point forward to
the ceiling that runs after them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerated by the Build Native Device workflow (run 31969160992) from the native sources on 'badrishc/optimize-device-iops'.
An audit of the review-driven hardening found two of its retry budgets
guard failure modes that cannot occur.

The startup QueueRun probe retried on the premise that a Linux backend can
answer with a transient negative when a signal interrupts the probing
thread. A zero timeout never blocks, so neither backend can: libaio passes
a zero io_getevents timeout and io_uring reads the completion queue in user
space without a syscall. A 3,000,000-iteration harness that hammered
io_getevents with a zero timeout while another thread signalled the caller
through a handler installed without SA_RESTART observed zero negatives and
zero EINTR. The probe is back to a single call.

The io_uring SQPOLL wakeup path gained a second backoff stage of 1000 one
millisecond sleeps. An enter carrying only IORING_ENTER_SQ_WAKEUP never
waits, so it cannot return EINTR, which io_uring_enter(2) documents only
for IORING_ENTER_GETEVENTS; every other error on that path is permanent.
Sleeping cannot turn such a failure into a success, and sq_lock and the
epoch are held throughout, so the stage only held the ring for a second
before reaching the same outcome. The bounded yield budget is restored.

Both comments now state the mechanism that actually applies.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerated by the Build Native Device workflow (run 31975517483) from the native sources on 'badrishc/optimize-device-iops'.
Dispose waited for in-flight IOs with an unbounded `while (TotalInFlight() != 0)
Thread.Yield();`. In-flight only returns to zero if the kernel completes every
accepted IO, so a completion that is never delivered spins there forever: an
unkillable teardown that pins a core and reports nothing.

Lost completions are reachable from several directions — a stalled device or
driver, a dropped CQE, or an io_uring ring whose SQPOLL thread has died, after
which the ring accepts submissions whose completions never arrive.

The drain now runs against a deadline. It sits orders of magnitude above any
legitimate drain: outstanding IOs are already queued in the kernel, and every
native call a lease is held across is individually bounded (the submit paths
unwind after a fixed yield budget, QueueRunFor takes a timeout), so reaching it
means completions are lost rather than slow. On expiry the count is logged and
teardown proceeds, which is safe because the drainers are cancelled and joined
before the handle is freed — no user callback can run during teardown — and
NativeDevice_Destroy cancels or waits for whatever the kernel still owns.

SpinWait replaces the bare Thread.Yield() so the normal microsecond drain stays
spin-fast while a drain that runs to the deadline does not pin a core.

Verified by injecting a phantom in-flight count with no matching completion:
before, Dispose did not return within 150s; after, it returns at the deadline.

file_linux.cc: comment only. The SQPOLL submit path noted that a later submit
redelivers the wakeup, which holds only while the poll thread is parked. Once it
is gone nothing redelivers. Failing those IOs individually would not restore
correctness — everything already in flight on that ring is lost with it, and the
kernel holds the only reference to their contexts (their user_data), so there is
nothing to enumerate. The comment now states that and points at the drain bound.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The published RESP matrix was measured by a version of the generator that ran
the GET sweep unconditionally after the MSET load, without checking that the
load had written the dataset. A key that was never written is answered from
memory with no device IO, and a miss is roughly 30x cheaper than a disk read,
so a partial load inflates the reported figure: on this array an unloaded store
reports over 150 M ops/sec against a true storage-bound 6.3 M, and a ~35%
shortfall alone accounts for the previously published 7.36 M.

Re-measured every cell with the load verification in place (the loader reports
99,876,864 of 100 M ops). The pinned peak is 6.96 M (uring, t=48) rather than
7.41 M, both backends now peak at t=48 instead of libaio climbing through t=64,
and the no-pin rows drop further, which widens the NUMA-pinning gap. The new
figures are also physically coherent with the neighbouring layers, which the
old ones were not: raw device ~8.4 M > KV ~7.8 M > RESP ~7.0 M.

Also tighten the generator's own short-load guard from 90% to 99% of DBSIZE.
The 90% floor still admitted an ~11% overstatement; at 99% the reported figure
is within ~1% of the fully-loaded value.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The published storage-bound table was measured against a build whose buffer
pool differs from the one this branch ships, so it does not reproduce. Every
row is replaced with a fresh trimmed-mean-of-3 measurement on the current
tree, and the peak claim drops from ~7.8 M (95% of fio) to ~6.3 M (77%).

The uring rows now use the smart ring default instead of an explicit
--device-io-contexts 32, which under-provisions at t=64; the default is
faster in every cell and makes the table match the documented command.

Also corrects two claims the new data contradicts: NUMA pinning is within
noise for the disk-bound scenario (it gates the RAM-served scenarios), and
the three benchmarks use different datasets, so their numbers do not form
an ordering.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… pipeline

SectorAlignedBufferPool keeps a per-(thread, class) chain of returned buffers
and spills to a lock-guarded depot once the chain reaches LocalCap. A caller
that rents many buffers before returning any therefore hits the depot for
everything past the first LocalCap rents, however small its actual working set.

KV.benchmark issues --batch-size (default 1024) reads per iteration before
draining, so its per-thread burst is 1024 buffers against a LocalCap of 128:
7 of every 8 rents and returns went through the depot. Instrumenting the pool
during a disk-bound run measured 147.4M depot pops against 49.3M local hits
(74.8% of gets) and 147.6M depot spills against 196.9M local pushes, with zero
cross-thread and zero large-class traffic - about 1.09B Monitor operations,
6.6 per KV operation. User CPU per operation was 2.92x the raw device path
(3.011us vs 1.033us) while kernel CPU per operation was lower, so the cost was
managed-side, not IO.

Raising LocalCap to 1024 admits the whole burst. Both consumers improve:

  KV.benchmark, 100M x 100B on 8xNVMe RAID-0, 100% random reads from disk,
  trimmed means of 3 (ops/sec):

    backend  pin      t=8         t=32        t=64
    libaio   node-0   2,366,537   6,861,507   7,707,696
    libaio   none     2,391,726   6,903,713   7,650,788
    uring    node-0   2,296,988   7,621,662   7,226,697
    uring    none     2,266,429   7,722,915   7,368,255

  Peak 7.72M against 6.34M before, +21.8%, and 94% of the array's 8.24M fio
  ceiling. Sweeping LocalCap alone at t=32 traces the burst exactly:
  128 -> 5.738M, 512 -> 6.786M, 1024 -> 6.942M, 2048 -> 6.948M, 4096 -> 6.910M,
  i.e. throughput follows min(LocalCap / 1024, 1) and flattens once the cap
  covers the batch.

  RESP GET, 3 rounds with the arms rotated each round, medians:

    LocalCap   t=48      t=64
    128        6.725M    6.423M
    1024       7.398M    7.216M
    2048       7.229M    7.182M

  The 128 and 1024 ranges are disjoint at both thread counts. 2048 is below
  1024, so 1024 is the value both consumers want.

Peak RSS falls: 9,135,776 kB at 128 against 8,930,012 kB at 1024. At the lower
cap the depot overflows and the pool drops and re-allocates buffers
continuously (dropped-because-full tracked fresh allocations one for one);
covering the burst collapses the fresh-allocation rate to near zero. The chain
is intrusive - it links through the buffer's own next pointer - so a larger cap
costs no structural memory, and LocalByteCap (32 MB per thread and class)
remains the bound that stops one thread parking the budget.

This reverses the 2026-08-15 measurement that kept main's 128. That study swept
128/256/512 and read +2.32% at 512, inside the "under 5%, keep main's setting"
band. It was under-ranged: the RESP scatter-gather path rents about 2000
buffers per network batch, so 512 covers only a quarter of the burst. The arms
that cross the burst threshold are worth 10-12%, well outside the band, so the
same rule now selects the change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The pool budgets bytes, but the owner-local chain was bounded by a count.
A count cannot bound a byte budget: one size class's buffer is up to 512x
another's, so the same count means 512x the bytes depending on which class a
thread happens to use. The count a thread actually needs is its IO pipeline
depth, which is a property of the caller, not of the pool.

The mismatch had a reachable consequence. `permitBytes` is reserved for the
life of a parked buffer, so per-(thread, class) retention was
min(LocalCap x classBytes, LocalByteCap). Summed over the 16 small classes at
LocalCap 1024 that is 302 MB against a 256 MB small sub-budget, so a single
thread could exhaust it. Once `TryReserve` fails, every thread runs
non-cacheable - every Get allocates and every Return frees - and nothing trims
the thread holding the bytes.

Replace both caps with a per-thread byte ceiling: the small sub-budget divided
by `ConcurrencySharding.ExpectedConcurrentThreads` (min(2 x cores, 64)), floored
at 1 MB. A thread's classes share that ceiling work-conservingly, so a
single-class thread gets all of it. At the ceiling, `TryMakeRoom` reclaims from
the class furthest above its equal share (max-min fair) and refuses the request
only if the requester is already at or above its own share, which also makes
self-eviction impossible. Victims are spilled to the depot, not dropped, so they
stay allocated, budgeted and reusable. `ThreadShard.activeClasses` is maintained
incrementally so the common single-class case tests the ceiling in O(1) - this
matters because a thread at steady state sits at the ceiling and enters that
path on every return.

Worst-case per-thread retention drops from 302 MB to 4 MB here (75x). The
smaller ceiling does not increase churn, because the real bound is the caller's
IO pipeline depth and both workloads sit under the slice (KV ~1.15 MB, RESP
~2.3 MB derived from their burst sizes and class mix).

Measured on 8xNVMe RAID-0, pinned, median-of-3, against the count-cap arm:
RESP libaio t=48/64 7.094/7.095 (was 6.923/7.167), uring 7.389/7.030 (was
7.291/7.013); KV libaio t=64 7.76 M; peak RSS 8,927,844 kB (was 8,930,012 kB).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The published figures were measured before the buffer pool's owner-local
retention was sized to a thread's IO pipeline, so they understated libaio by
up to 7% (pinned t=48: 6.51 -> 6.97 M) and reported the wrong peak thread
count for libaio.

Re-measured the full 16-cell out-of-box matrix with the generator script
(median of 3, load verified at 99,876,864 of 100 M keys) and republished
every cell:

  backend  NUMA          t=8     t=32    t=48    t=64
  Libaio   srv-0/cli-1   1.82    5.70    6.97    7.06
  Libaio   no pin        1.38    5.09    5.72    5.57
  Uring    srv-0/cli-1   1.82    6.13    7.32    6.89
  Uring    no pin        1.48    4.60    5.55    6.29

Derived claims corrected along with the numbers:

- Peak is 7.32 M (uring, pinned, t=48), ~89% of the array's fio ceiling, not
  7.0 M / ~84%.
- The backends no longer peak at the same thread count: uring peaks at t=48
  and eases off at t=64, while libaio is still climbing at t=64. The guidance
  is now to sweep the t=48-64 band.
- The backend-parity bullet quantifies the spread (uring leads 5-7% at
  t=32-48, libaio by 2% at t=64) instead of claiming "within a few percent",
  which the t=32 cell no longer supports.
- The tuned-vs-default claim drops its numeric bound, which was not
  re-measured on this binary.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The published matrix shows uring ahead of libaio by 7.5% at t=32 and 5.0%
at t=48, and libaio ahead by 2.5% at t=64. State 5-8% and ~2% so the
bullet matches the table above it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The website design doc still described LocalCap (128 buffers) and LocalByteCap (32 MB) bounding a single (thread, size-class) local stack. Neither constant exists: local retention is now bounded in bytes per thread, shared across that thread's size classes, at smallBudget / ExpectedConcurrentThreads floored at MinThreadLocalBytes (4 MB at the 1 GiB default), with max-min fair admission across classes via TryMakeRoom.

Also correct the summary's '8-way' depot striping, which contradicted section 6 (8-64 stripes, sized from the processor count), and note that a spill relocates a buffer to the depot with its permit intact rather than making it uncacheable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The depot section justified its lock against ConcurrentStack point by point, and the large-class section closed with before/after reuse, allocation and RSS figures from an earlier iteration. Both read as defenses of past decisions rather than a description of the design.

Keep every substantive fact - atomic close, exact capacity bound, allocation-free push, and why large classes have no per-thread locality to exploit - and state them as properties of the final design. Align the equivalent source comment in ReturnOriginReturn.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On the SQPOLL submit path the retry loop re-enters io_uring_enter until the
wakeup is delivered or the yield budget is exhausted. On exhaustion the entry
is already published to the kernel, so the operation is reported submitted and
the loop's last errno was discarded, leaving a ring that can accept IOs whose
completions never arrive with no signal to the operator.

Emit the errno and its consequence once per device, claimed through an atomic
flag so a ring that fails for every submission reports a single line rather
than one per IO. The report does not change the submitted outcome: the entry is
kernel-owned and the kernel holds the only reference to the affected contexts
via their user_data, so there is nothing to enumerate and rewriting the SQE
would race the poll thread. NativeStorageDevice.Dispose already bounds its
drain, so a ring in this state cannot hang teardown.

Rebuilt the linux-x64 uring binary. The libaio variant is unchanged: this code
is inside the FASTER_URING guard, which that build does not define.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerated by the Build Native Device workflow (run 32199655122) from the native sources on 'badrishc/optimize-device-iops'.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 45 out of 58 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

libs/storage/Tsavorite/cs/benchmark/Device.benchmark/Program.cs:231

  • The benchmark's renamed --device-throttle-limit still does not configure LocalMemory's actual in-flight bound: this options object leaves RingCapacity at 0, which CreateLogDevice converts to 1024, while setting device.ThrottleLimit later has no effect because LocalMemoryDevice does not implement Throttle(). Thus the documented --device-throttle-limit 8192 LocalMemory run still uses a 1024-entry ring. Map the requested limit to a power-of-two ring capacity here, as KV.benchmark does.
                    localMemoryDeviceOptions: new LocalMemoryDeviceOptions { SegmentSize = segSize });

benchmark/Resp.benchmark/Program.cs:336

  • --load-threads accepts zero or negative values, but LoadData immediately evaluates DbSize % loadDbThreads and divides by it. A user-provided zero therefore crashes the benchmark with DivideByZeroException instead of a configuration error. Validate this new option before calling LoadData.
                    bench.LoadData(loadDbThreads: opts.LoadThreads, keyLen: keyLen, valueLen: valueLen, numericValue: opts.Op == OpType.INCR);

Comment thread libs/storage/Tsavorite/cs/src/core/Device/NativeStorageDevice.cs
Comment thread libs/storage/Tsavorite/cc/src/device/file_linux.cc Outdated
… two benchmark option bugs

Addresses the fresh Copilot review on #2018.

NativeStorageDevice.Dispose(): the shard in-flight counter was overloaded for
two things with different termination properties — IOs awaiting a completion
(which can be lost forever, hence the bounded drain) and leases held by threads
executing inside native code. TryComplete/TryCompleteMine hold a lease across a
native call that dispatches user callbacks inline, so a lease is bounded only by
user code, not by the "every native call is bounded" claim the drain comment
made. A lost completion could therefore trip the deadline while a native frame
was still running, and the subsequent NativeDevice_Destroy would free its rings
and locks underneath it.

Leases are now counted separately (a second field on the existing padded shard
counter, so no extra cache miss) and handle destruction waits on that counter
after the in-flight drain. Leases are bumped after in-flight and dropped before
it, so leases <= in-flight always holds and the normal path costs one extra
read. No new lease can be acquired once disposedFlag is published, so the wait
only covers calls already in native code; if it does expire the handle is leaked
rather than freed, which is bounded and diagnosable where a use-after-free is
not.

Device.benchmark: --device-throttle-limit had no effect on LocalMemory.
LocalMemoryDevice does not override StorageDeviceBase.Throttle() (which returns
false), so its in-flight bound is the per-submitter SPSC ring, and RingCapacity
was left at 0 => 1024. Map the throttle onto the ring capacity as KV.benchmark
already does, and print the resolved value.

Resp.benchmark: --load-threads 0 reached DbSize % loadDbThreads and crashed with
DivideByZeroException; reject values below 1 up front.

file_linux.cc is comment-only: state that io_uring_sq_ready() is sqe_tail-khead,
measured against the kernel head rather than ktail, so submit's flush cannot
drive it to zero and it remains an exact "kernel consumed our SQE" test under
short submits and failed enters. No behavior change, so the prebuilt binaries
are unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

3 participants