gortiz opened a new pull request, #19021:
URL: https://github.com/apache/pinot/pull/19021
## Add a Foreign Function & Memory (Panama) based PinotDataBuffer
implementation
### Motivation
Pinot's long-indexed (>2 GB) off-heap buffers are currently backed by
`sun.misc.Unsafe`
(`UnsafePinotBuffer` + `MmapMemory`, with `chronicle-core` reflection for
`mmap`). `Unsafe` is
deprecated and slated for removal, its `mmap` path reflects into
JDK-internal `sun.nio.ch` classes
(requiring `--add-opens` and failing without them), and it does not work on
Windows.
Now that Pinot builds on JDK 25, the Foreign Function & Memory API
(`java.lang.foreign`, finalized
in JDK 22) is a first-class replacement: `MemorySegment` supports `long`
indexing natively, is
memory-safe, needs no reflection or `--add-opens`, and works on every OS.
This PR adds an **opt-in** FFM-backed `PinotDataBuffer`. **The default is
unchanged** (`Unsafe`
wrapped in `SmallWithFallback`); the new factory is selected only via the
existing
`pinot.offheap.buffer.factory` config or the `PINOT_BUFFER_LIBRARY`
environment variable.
### What's in this PR
- **New module `pinot-segment-spi-foreign`** (Java 25):
- `ForeignPinotBuffer` — native-order `PinotDataBuffer` over
`MemorySegment` (`*_UNALIGNED`
layouts, `asSlice` views, `asByteBuffer` borrowed views,
`MemorySegment.copy` bulk get/put,
`force()` flush).
- `ForeignPinotBufferFactory` — `Arena.ofShared()` for `allocateDirect`,
`FileChannel.map(mode, offset, size, arena)` for `mapFile` (no
reflection, no page-alignment
handling, no `chronicle-core`, long-sized maps directly). Non-native
order reuses
`NonNativePinotDataBuffer`.
- Wired into `pinot-bom`, the root reactor, and depended on by
`pinot-segment-local` so it is on the
server/broker runtime classpath and bundled in the distribution.
- `pinot-perf` benchmark updates (below).
### Why a separate module (not `pinot-segment-spi`)
`pinot-segment-spi` is intentionally pinned to Java 11 bytecode because it
is in the JDBC/Java driver
dependency closure (`pinot-jdbc-client → pinot-java-client → pinot-common →
pinot-segment-spi`),
which must keep running on JDK 11. `java.lang.foreign` requires Java 21+, so
it cannot live there. A
dedicated Java 25 module preserves the driver's Java 11 guarantee — the
dependency edge points *into*
the new module, never out of the driver closure. (Verified:
`pinot-segment-spi` bytecode stays major
version 55 / Java 11; the new module is major version 69 / Java 25.)
### Zero-copy `ByteBuffer` views preserved (important for RoaringBitmap et
al.)
`PinotDataBuffer.toDirectByteBuffer(...)` must return a **view**, not a
copy, of a buffer region — it
is what lets consumers such as RoaringBitmap read a region *in place* (e.g.
via
`viewAsRoaringBitmap`, which wraps the returned `ByteBuffer` in an
`ImmutableRoaringBitmap`) with no
allocation on the query path, straight off the mmap'd file pages.
The FFM implementation keeps this zero-copy behavior via
`MemorySegment.asByteBuffer()`, which
returns a direct `ByteBuffer` aliasing the same native memory (verified: the
view is direct, writes
are visible in both directions, and the `ByteBuffer`'s native address equals
the segment's). This is
actually cleaner than the `Unsafe` path, which produces the same view by
*reflecting* into private
`DirectByteBuffer` constructors with per-JDK-version fallbacks
(`ByteBufferUtil`) — all of that
reflection is unnecessary with FFM. The view is scope-bound ("borrowed"):
valid while the backing
`Arena` is alive, and using it after close throws `IllegalStateException`
instead of `Unsafe`'s
undefined behavior. A dedicated test
(`testToDirectByteBufferIsZeroCopyView`) locks this in.
### Backward compatibility
- **No behavioral change by default.** No public API changes. Opt-in factory
FQN unchanged, so
existing `pinot.offheap.buffer.factory` / `PINOT_BUFFER_LIBRARY` values
keep working.
### Performance
JMH, macOS / JDK 25, single fork — representative results (numbers are from
a developer laptop and
have run-to-run variance; the takeaways, not the exact figures, are the
point). **Bottom line:
read/write is at parity with `Unsafe` on both micro and end-to-end query
benchmarks; the only
regression is one-time allocation, which only affects the >2 GB path and
amortizes to zero for
long-lived segment buffers.**
**1. Read/write hot path (`BenchmarkPinotDataBuffer`, ns/op, value length
1024):**
| op | unsafe | foreign | bytebuffer |
|----|-------:|--------:|-----------:|
| batchRead | 29.6 | 30.0 | 29.6 |
| batchWrite | 22.9 | 22.3 | 22.6 |
| nonBatchRead | 230 | 192 | 26 |
| nonBatchWrite | 210 | 184 | 27 |
FFM matches `Unsafe` on bulk copy and is a bit faster on per-element access.
(`ByteBuffer` is faster
still for per-element reads, but is capped at 2 GB — it cannot back the
buffers this factory exists
for.)
**2. Bulk copy required an override.** With the base
`copyTo`/`readFrom(byte[])` (which allocates a
throwaway `ByteBuffer` per call), foreign `batchRead(1024)` was **41.5
ns/op**; overriding them with
`MemorySegment.copy` brought it to **30.0 ns/op** — parity with `Unsafe`.
**3. Allocation is slower — profiled to explain why.** Isolated
allocate-vs-close, 1 MB buffer, per
op:
| | allocate | close |
|-|---------:|------:|
| foreign (FFM) | 14.6 µs | 7.0 µs |
| unsafe | 9.5 µs | 0.02 µs |
| unsafe (no zero-fill) | 0.02 µs | 0.02 µs |
The 1 MB zero-fill (~9.5 µs) is paid by **both** (it is essentially all of
`Unsafe`'s allocate time —
`malloc` reuses the warm freed block). The foreign gap decomposes into **~7
µs close** (async-profiler
shows ~82% of allocate time is `Arena.ofShared().close()`, which performs a
JVM-wide
`VM_HandshakeAllThreads` to guarantee no thread is mid-access before
freeing) plus **~5 µs allocate**
(constructing the `MemorySegment` + shared `MemorySessionImpl` + `Cleaner`
registration). For tiny
buffers the gap is larger relatively (~6.4 µs vs ~0.28 µs) as there is no
zero-fill to amortize
against. This is a **one-time, per-buffer cost** on the >2 GB path only (the
default
`SmallWithFallback` routing keeps ≤2 GB buffers on `ByteBuffer`), where
buffers are long-lived and
allocated/freed once — so it does not surface on the query path.
**4. End-to-end queries (`BenchmarkQueriesSSQE`, 4M rows across mmap'd
segments):** `foreign`,
`unsafe`, and `bytebuffer` are all within run-to-run noise of each other
across scan-heavy and
group-by queries — confirming no query-time regression, since segments are
mmap'd once and read many
times.
### Testing
- New `ForeignMemoryPinotDataBufferTest` runs the full existing
`PinotDataBufferTest` contract
(including the >2 GB `LARGE_BUFFER_SIZE` allocate/loadFile/mapFile cases
in both byte orders), plus
FFM-specific tests: zero-copy `ByteBuffer` view semantics, use-after-close
fails deterministically
(`IllegalStateException`), `long`-offset addressing beyond 2³¹, and
read-only-mapped writes
rejected (`IllegalArgumentException`). 19 tests, all passing on a clean
build.
- `pinot-perf`: `BenchmarkPinotDataBuffer` gains `foreign`/`wrapper+foreign`
options;
`BenchmarkQueriesSSQE` gains an opt-in `_bufferLibrary` parameter
(defaults to `default`, so the
standard matrix is unaffected). Both benchmark forks now pass the standard
`--add-opens` set so the
`Unsafe` factory (which needs them for its `mmap` path) can be benchmarked.
### Notes
- Requires a JVM ≥ 21 to load (Java 25 bytecode); only reachable when
explicitly selected, and Pinot
services run on JDK 25.
- No new JVM flags are required for the FFM path (`Arena` / `MemorySegment`
/ `FileChannel.map` are
not restricted methods).
release-notes: Add an opt-in `java.lang.foreign` (Panama) based off-heap
buffer implementation,
selectable via `pinot.offheap.buffer.factory` / `PINOT_BUFFER_LIBRARY`. The
default (Unsafe/ByteBuffer)
is unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
