GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||
GeezerVison is showing github.com — Fix #215: correct NIO buffer for Netty ByteBuf reads (nioBufferView + nioBufferCopy) by bernardladenthin · Pull Request #290 · lmdbjava/lmdbjava Conversation
|
nioBufferView(ByteBuf) |
nioBufferCopy(ByteBuf) / nioBufferCopy(ByteBuf, IntFunction<ByteBuffer>) |
|
|---|---|---|
| mechanism | zero-copy alias of LMDB mmap (same Unsafe address/capacity trick as ByteBufferProxy), via a reused per-thread base buffer |
copy via ByteBuf.getBytes into a new direct buffer, or into a caller-supplied (e.g. pooled) buffer |
| cost | no copy; one read-only wrapper object per call | one copy |
| lifetime | valid only while the txn is open (misuse → SIGSEGV, documented) | outlives the txn/Env close |
requires --add-opens java.base/java.nio |
yes (lazily, only if called) | no |
Commits:
test(#215) — repro test pinning the current (buggy) behaviour.feat(#215) — nioBufferView (zero-copy) + test.feat(#215) — nioBufferCopy (safe copy) + tests.feat(#215) — review: nioBufferCopy supplier overload; nioBufferView reuses a per-thread base buffer.ByteBufNioBufferTest (9 tests): view/copy reflect data, copy survives txn+env close, earlier views unaffected by later calls, supplier contract (exact size, position/flip, untouched tail, too small → IAE, null → NPE), raw nioBuffer() bug documented.mvn verify and fmt:check clean; new code fully covered.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…le data A value ByteBuf returned via ByteBufProxy.PROXY_NETTY reads correctly through the ByteBuf's own accessors, but ByteBuf.nioBuffer() does not reflect the stored data (it views Netty's separate, chunk-shared backing buffer, which the zero-copy read path never repoints). This test pins the current behaviour; the following commits add helpers to obtain a correct NIO buffer. Refs lmdbjava#215. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01GdiZ3ABYsVHXBRNCkEizpE
Adds a static helper that materialises a read-only NIO ByteBuffer aliasing the LMDB memory referenced by a PROXY_NETTY value ByteBuf, using the same Unsafe address/capacity technique ByteBufferProxy already uses for its own zero-copy NIO buffers. This lets Netty users hand a correct NIO buffer to NIO-based consumers (compression, hashing, ...) instead of getting the zeros that ByteBuf.nioBuffer() returns. The java.nio.Buffer field offsets are resolved lazily in a holder so a JVM lacking --add-opens java.base/java.nio only fails if nioBufferView is actually called, rather than breaking PROXY_NETTY initialisation for everyone. The returned buffer aliases LMDB-owned, read-only memory and is valid only while the owning read Txn is open — documented prominently, as misuse is a JVM crash. Refs lmdbjava#215. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01GdiZ3ABYsVHXBRNCkEizpE
…ative Adds a second static helper as an alternative to nioBufferView: instead of aliasing LMDB memory, it copies the readable bytes out via the ByteBuf's own (correct) accessor into a fresh direct NIO buffer. This uses no Unsafe/reflection, needs no --add-opens, and — unlike the view — remains valid after the owning transaction (or the Env) is closed. The two helpers are deliberately offered together so the maintainers can choose: keep the zero-copy view, the safe copy, or both. Dropping this commit removes the copy variant; dropping the previous commit removes the view. Recommendation: nioBufferCopy as the safe default, nioBufferView for zero-copy hot paths. Refs lmdbjava#215. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01GdiZ3ABYsVHXBRNCkEizpE
|
Thanks for the PR. I don't see any reason not to add both of these utility methods. However, I think public static ByteBuffer nioBufferCopy(final ByteBuf buffer) public static ByteBuffer nioBufferCopy( final ByteBuf buffer, final IntFunction<ByteBuffer> destinationBufferSupplier) This allows the caller to use a buffer they already have or can obtained from a pool under their control. Similarly, |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
… reuse view base Adds nioBufferCopy(ByteBuf, IntFunction<ByteBuffer>) so callers can copy into a buffer they already have or obtain from their own pool. The supplier is given the readable size and must return a buffer with at least that many bytes remaining; the bytes are written at its position and the buffer is returned flipped. The one-arg form now delegates with ByteBuffer::allocateDirect. nioBufferView no longer allocates a zero-length direct buffer per call: it repoints a per-thread base buffer (ThreadLocal, as in ByteBufferProxy) and returns asReadOnlyBuffer(), which snapshots the address/capacity, so earlier views are unaffected by later calls on the same thread. Refs lmdbjava#215. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01L4Qkne1apX58veQ6bY3w84
|
Thanks @at055612 — done in 80b8abf:
5 new tests in |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
ByteBuf.nioBuffer() returns a buffer with all zeros
There was an error while loading. Please reload this page.