Skip to content

perf: optimize decoded MultiGet reads - #24

Merged
svlachakis merged 21 commits into
mainfrom
perf/multiget-bench
Oct 16, 2026
Merged

svlachakis merged 21 commits into
mainfrom
perf/multiget-bench

Conversation

@svlachakis

@svlachakis svlachakis commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Add decoded fixed-length MultiGet overloads that write into a caller-owned span. The generic overload supports struct decoders without boxing. The interface overload forwards to it and supports the same sorting hint. This includes the API proposed in #17; its separate implementation is not needed.

One key uses the existing pinned-read helper. Two keys use the existing batched C API. Larger batches share one native owner and avoid per-value wrapper allocations and native getter/destroy calls. All-missing batches skip owner allocation. Missing and empty values remain distinct. Database and read-options leases stay held until native cleanup completes.

Non-default read tiers use scalar reads for every family. A scalar read failure retains the decoded prefix and leaves the tail cleared. Native batch errors are checked before decoding. Disposed family handles are rejected at entry and before each scalar read. Native exceptions cannot escape the new batch C API.

The PR contains the managed API, native extension, six rebuilt native libraries, regression tests and API documentation. Build dependencies and CPU requirements are unchanged. The comparison harness, frozen references and proof scripts are outside this PR.

Validation

  • All six native builds passed in 38890192099, built at cb3bcdc. Every packaged LFS object at 1b400f9 matches its verified build artifact. The removed profiler export is absent from all six libraries.
  • Packaged-library tests 38892870090 passed on all six platforms at 1b400f9. The expanded suite has 549 tests on Linux. The solution compiles with zero warnings and errors.
  • Final proof 38892956847 passed on AMD64 and ARM64. It checked out exact packaged revision 1b400f9, verified native sources, build results and binary hashes, then ran packaged tests and measurements. Each architecture has 117 complete BenchmarkDotNet results with two launches, plus three paired processes.

Measurements

Time change of the new generic overload with a struct decoder. Negative means less time. Each cell is the median of three process medians. Each process uses twelve balanced ABBA/BAAB rounds on one pinned CPU, with matched A/A controls. * means at least one process did not separate from zero and its control variation; it is not a confirmed gain. The bands describe observed variation, not confidence intervals.

AMD64: AMD EPYC 7763. ARM64: Neoverse-N2. Baselines: main 165c9d2, #17 d6040cf, previous #24 f189a51. All methods use the same rebuilt native library. Main is the existing array-returning API; the candidate uses prepacked contiguous keys and a caller-owned output span. BenchmarkDotNet also measures key-packing costs separately.

Unless stated otherwise, cases use 32-byte keys, 128-byte values, 100% hits, a named column family and warm SST data.

Case AMD64 vs main AMD64 vs #17 AMD64 vs prior #24 ARM64 vs main ARM64 vs #17 ARM64 vs prior #24
1 key -35.20% -29.22% -1.56%* -31.94% -24.75% -0.75%
2 keys -9.37% -1.38% -0.38%* -12.46% -2.65% -0.71%*
8 keys -11.29% -3.27% -1.78% -13.76% -4.52% -2.43%
32 keys -12.41% -3.63% -2.18% -15.58% -4.91% -2.92%
256 keys -14.00% -4.44% -3.56% -18.01% -6.48% -4.63%
512 keys -14.04% -5.23% -3.73% -18.52% -7.92% -5.77%
32 keys, all missing -12.70% -1.25% -0.73% -16.26% -1.93% -1.11%
32 keys, 50% hits -11.82% -2.78% -1.56% -15.03% -4.02% -2.25%
32 keys, 8 B values -9.68% -3.92% -3.38% -12.26% -4.66% -2.73%
32 keys, 8 KiB values -61.69% -3.94% -2.85% -68.80% -5.60% -2.90%
32 keys, memtable -16.68% -5.59% -3.72% -22.27% -6.95% -4.52%
32 keys, default CF -11.86% -33.70% -3.73% -15.29% -29.02% -3.13%
512 keys, sorted -23.64% -14.91% -4.02% -26.46% -15.91% -6.14%

The struct decoder took less time than main in all thirteen cases, beyond the matched controls in all three processes on both architectures.

The two-key class decoder has a small measured time increase versus the previous #24: the median of process medians is +0.91% on AMD64 and +0.14% on ARM64. Two AMD64 processes and one ARM64 process exceeded their control bands; the other class two-key processes were inconclusive. The struct two-key changes versus the previous #24 were also inconclusive. These results do not support a blanket claim of no performance regressions versus the previous implementation.

The 7,488 candidate samples in the paired comparisons recorded zero managed allocations and zero GC collections. BenchmarkDotNet also reported zero managed bytes for all Candidate, CandidateClass and CandidatePacked cases on both architectures. In the 32-key paired case, main allocated 228.625 managed bytes per key; the new overload allocated zero. These allocation results apply to the measured numeric decoders and warmed paths.

Raw data: AMD64, ARM64.

Callers must use the new overloads to get these gains. The existing allocating overload is unchanged. These are warm component benchmarks; they do not establish RPC or block-processing gains. The native owner retains value pins until decoding completes. Slow decoders and cache pressure are outside this matrix.

Compare unchanged main, PR #17 and constrained decoding. Include key packing, A/A controls and two launches on AMD64 and ARM64.
Keep the database lease through decoding without acquiring it twice.

Restore scalar error scanning: paired AMD64 results did not show a
consistent benefit from the span scan.
Keep one decoded implementation and forward the interface overload.
Move the comparative harness to perf/multiget-bench-proof.
Cover full payloads and document sorting and native value lifetimes.
@svlachakis svlachakis changed the title perf: add decoded MultiGet fast paths and benchmarks perf: add decoded MultiGet overloads Oct 15, 2026
@svlachakis svlachakis changed the title perf: add decoded MultiGet overloads perf: optimize decoded MultiGet reads Oct 16, 2026

@benaadams benaadams left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-review at 6f729e0. The earlier findings are resolved:

  • The native batch path is now the stated design.
  • The harness and the experiment workflows are gone.
  • All six native libraries were rebuilt: each sha256 matches its LFS oid against the artifacts of run 38880619766, built at d97799e. No native source or build workflow changed between that build and the head.
  • Tests are green on all six platforms in run 38885742490.

There is one Medium, inline: the cache-only read tier is silently ignored for named families or an explicit default handle. The rest is Low or Nit.

Not inline:

  • Test gaps:

    • No present-but-empty value goes through the 2-key path. _WithMultiByteValues writes [] only when count > 2, and _WithOnlyMissingOrEmptyValues starts at 3.
    • CountingValueDeserializer only runs under SetReadTier(1), so no stateful struct decoder reaches either batched loop.
    • _WhenDecoderClosesDatabase doesn't cover 2 keys or a named family.

    Each of these lets a regression pass, for example skipping zero-length values on the 2-key path, or copying the struct decoder by value inside a loop.

  • checked(count * 2) throws OverflowException above 2^30 values after the output has been cleared. That exception, and the InvalidOperationException from the native path, are undocumented. The interface overload also cannot pass sortedInput.

  • AGENTS.md:9 says the native declarations are generated, but the batch header is bound by hand. The git-lfs install at build-rocksdb.yml:50 looks unused, and the reason for the safe.directory step (:73-78) is not stated.

  • The PR body still says the comparison table is "pending" and "CI is being repeated". It cites run 38884441416, which is at 391ccbd, not the head. It also has no measurements for a perf: change.

Comment thread native/rocksdb_batch.inc Resolved
Comment thread native/rocksdb_batch.inc Resolved
Honor non-default read tiers for every column family. Reject disposed handles before native reads, including decoder reentry. Contain native exceptions and remove profiling-only code.

@benaadams benaadams left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All review points are addressed at cb3bcdc:

  • The read tier is honoured for every family on both batched paths.
  • Disposed family handles are rejected, both at entry and before each scalar read.
  • The jemalloc probe is gone.
  • Native exceptions are contained.
  • The generic cases run a struct decoder.
  • The exception docs and the sortedInput overload are in.

One thing to land before merge: native/rocksdb_batch.inc and rocksdb_batch.h changed in this commit, but the six packaged libraries are still the ones built at d97799e (run 38880619766), so they still export rocksdb_net_thread_allocated and lack the catch (...). Please commit the binaries from run 38890192099 and re-run the packaged tests on them.

@svlachakis

Copy link
Copy Markdown
Contributor Author

The rebuild is already in progress in run 38890192099. Five platforms have passed; macOS x64 is still building. I'll commit all six binaries and rerun the packaged tests when it finishes.

@svlachakis

Copy link
Copy Markdown
Contributor Author

All six binaries from 38890192099 are now committed at 1b400f9. Their hashes match the build artifacts, and the packaged tests passed on all six platforms in 38892870090. The native threads are resolved. The final benchmark is running on this revision in 38892956847.

@svlachakis
svlachakis merged commit 94e9fea into main Oct 16, 2026
11 checks passed
@svlachakis
svlachakis deleted the perf/multiget-bench branch October 16, 2026 11:47
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