mdbx: Cursor.GetBatch — batched cursor reads (one cgo call per N records, ~3.5x faster scans) - #240
mdbx: Cursor.GetBatch — batched cursor reads (one cgo call per N records, ~3.5x faster scans)#240AskAlexSharov wants to merge 21 commits into
Conversation
…records Full-table iteration through Cursor.Get pays the fixed Go->C call overhead once per record, which dominates the cost of small-record scans. GetBatch moves the loop into C: one cgo call fills a reusable C-allocated buffer of MDBX_val pairs (C memory so the cgo runtime never scans it for Go pointers), and Go reads the results as zero-copy views. Benchmark (100k records, 12-byte keys/values, darwin/arm64): Get_Next 34.2 ns/record GetBatch_64 10.3 ns/record GetBatch_256 9.7 ns/record GetBatch_1024 9.7 ns/record ~3.5x faster full scans; the returned slices have the same lifetime rules as Cursor.Get (valid until txn end / next batch reusing the buffer).
3052e1f to
6a95848
Compare
There was a problem hiding this comment.
Pull request overview
Adds a batched cursor-scan API to reduce per-record cgo overhead by moving the inner mdbx_cursor_get loop into C and returning zero-copy key/value views via a reusable C-allocated buffer.
Changes:
- Add
mdbxgo_cursor_get_batchC helper that retrieves up to N(key,value)pairs per call. - Introduce
GetBatchBuffer+Cursor.GetBatchGo API backed by C-allocatedMDBX_valstorage. - Add correctness/no-alloc tests and a benchmark comparing
Get/NextvsGetBatch.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| mdbx/mdbxgo.h | Declares the new mdbxgo_cursor_get_batch helper and documents its contract. |
| mdbx/mdbxgo.c | Implements the batched loop around mdbx_cursor_get and writes results into a caller-provided MDBX_val pair buffer. |
| mdbx/batch.go | Adds the public Go API (GetBatchBuffer, Cursor.GetBatch) and C-memory buffer management. |
| mdbx/batch_test.go | Adds tests for correctness/allocations and a benchmark for scan performance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…op constraints, reset benchmark timers
…-on-error 2*(1<<28)*sizeof(MDBX_val) wraps a 32-bit size_t; 1<<20 pairs keeps the byte count below 2^25 everywhere and is still far beyond useful batch sizes. GetBatch doc now states the first n pairs stay valid on error.
…ndary paths Indexing past the pairs actually filled returned stale views from an earlier fill (possibly of an ended txn) instead of failing; at() now panics past buf.n. Adds exact-fill boundary and partial-on-error tests, documents *_MULTIPLE page granularity and write-txn view invalidation, and reorders the doc example to consume n before handling err.
…mpty key is NOTFOUND, not an error
An early EINVAL return left buf reporting entries from a previous fill, so Key/Val could hand out views of an already-ended transaction.
The batch loop stopped on any non-MDBX_SUCCESS return, but MDBX_RESULT_TRUE (e.g. a lower/upper-bound reposition) is success with a valid key/value. Treating it as a stop dropped that pair and, once operrno mapped RESULT_TRUE to nil on the Go side, could surface (n=0, eof=false, err=nil). The C loop now stores the pair and continues for RESULT_TRUE, same as SUCCESS.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
mdbx/mdbxgo.c:126
mdbx_cursor_getcan returnMDBX_RESULT_TRUE(success-with-data). This helper treats it as success inside the loop, but it may returnr.err == MDBX_RESULT_TRUEwhen the buffer fills (e.g. max_pairs==1 on an inexact SetLowerBound), which callers may interpret as an error. Normalize the return code toMDBX_SUCCESSwhen the batch completes successfully.
r.err = mdbx_cursor_get(cur, &key, &val, op);
/* MDBX_RESULT_TRUE (e.g. a lower/upper-bound reposition) is success
* with a valid pair, not a stop condition: store it and continue. */
if (r.err != MDBX_SUCCESS && r.err != MDBX_RESULT_TRUE)
break;
pairs[2 * r.val] = key;
pairs[2 * r.val + 1] = val;
r.val++;
op = op_next;
}
return r;
…der contract The Go side now handles RESULT_TRUE in the same success arm as SUCCESS (it was already correct via operrno's RESULT_TRUE->nil mapping, but read as if RESULT_TRUE were an error). Updated the mdbxgo.h contract comment to note RESULT_TRUE and that a mid-batch error still leaves r.val valid pairs stored.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
mdbx/batch.go:105
- GetBatch docs say ops that require a search key/value (Set, SetRange, GetBoth, …) cannot be used, but the implementation currently passes an empty key/value down to mdbx_cursor_get. That makes these ops behave like “search for empty key/value”, which is both surprising and contrary to the stated contract. Consider rejecting such ops up-front with EINVAL so misuse is caught early and behavior matches the documentation.
if c._c == nil || buf == nil || buf.ptr == nil || buf.size <= 0 {
return 0, false, operrno("mdbx_cursor_get", C.MDBX_EINVAL)
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
mdbx/batch.go:56
- GetBatchBuffer.at() panics with “index out of range…” both for an out-of-range index and for use-after-Close (b.ptr == nil). Splitting the checks provides a clearer panic cause when the buffer has been closed.
if b.ptr == nil || i < 0 || i >= 2*b.n {
panic("mdbx: GetBatchBuffer: index out of range of the last GetBatch fill")
}
…ill count GetBatch passes an empty key/value to mdbx_cursor_get, so an op that expects one silently produced a plausible wrong answer instead of an error: (Set, Next) matched nothing and reported a clean EOF, (First, Set) ended the scan after one record the same way, and (SetRange, Next) rewound to the first key. Restrict opFirst/opNext to the ops that need no input — the start_op/turn_op sets libmdbx documents for mdbx_cursor_scan, plus the turn ops as opFirst so a scan can continue past its first batch — and return EINVAL for the rest. The allow-list also rejects GetCurrent as opNext (it would restream one record until the buffer filled) and out-of-range op values. Close now clears the last-fill count as well, so Key/Val panic with "use after Close" instead of handing out views of freed memory; at() reports that case separately from an out-of-range index. Tests: rejected-op table, reverse (Last, Prev) scan, DupSort (FirstDup, NextDup) scan, use-after-Close and out-of-fill panics. The partial-on-error test now provokes MDBX_INCOMPATIBLE with NextMultiple, which stays valid under the allow-list. Scan benchmark is unchanged: 36.6 ns/record for Get/Next vs 9.2 for GetBatch(256).
…ixture The rejected-op assertions checked IsErrnoSys(err, syscall.EINVAL), which only holds on POSIX: on Windows MDBX_EINVAL is ERROR_INVALID_PARAMETER, so every case failed the win job. Pin the rejection itself — an error, n=0, eof=false — as TestTxn_Reset_ReturnsError already does for the same reason. fillBatchDB uses fmt.Appendf, fixing the two modernize findings from lint.
Scanning via
Get(Next)pays the fixed cgo overhead once per record.GetBatchmoves the loop into C: one call fills a reusable C-allocatedMDBX_valbuffer (never scanned for Go pointers),Key(i)/Val(i)return the usual zero-copy views, andeofdistinguishes exhaustion from buffer-full. Advancing ops only — there is no way to pass a search key, so for ranged scans position the cursor withGetfirst and batch with(GetCurrent, Next); DupSort works via(FirstDup, NextDup)etc.100k-record scan, darwin/arm64: 34.2 ns/record → 10.3 (batch 64) / 9.7 (batch 256+), ~3.5x. Includes correctness + no-allocs tests and the benchmark.