feat(jni): bridge caller-provided Java I/O into VortexReadAt/VortexWrite

[robert3005]

Implement VortexReadAt and VortexWrite using jvm native interfaces to be able to use java readers and writers natively in vortex

[0ax1]

@claude review

[github-actions]

Claude finished @0ax1's task in 1m 45s —— View job

---

Claude PR Review

Reviewed the JNI I/O bridge (vortex-jni/src/io/, data_source.rs, writer.rs, errors.rs) and the Java contract interfaces. This is a clean, well-documented bridge — the threading/JVM-lifetime docs in io/mod.rs are excellent and the read-side concurrency limiter is thoughtfully designed. A few correctness and performance points below.

Correctness

1. Integer overflow in the read bounds check — io/read_at.rs:136

let end = offset + length as u64;
if end > len {
    vortex_bail!("read {offset}..{end} out of bounds ...");
}

offset + length can wrap. In release builds a wrapped end slips past the end > len guard (the check it exists to enforce); in debug it panics inside spawn_blocking rather than surfacing a clean VortexError. Prefer a checked add:

let end = offset.checked_add(length as u64)
    .ok_or_else(|| vortex_err!("read {offset}+{length} overflows u64"))?;

Fix this →

2. Write upcalls block a runtime worker thread — io/write.rs vs read_at.rs
Reads are dispatched via handle.spawn_blocking(...), but JavaWrite::write_all/flush call into Java inline on the task thread (session.handle().spawn(...) in writer.rs:295). A blocking Java write/flush (e.g. an Iceberg PositionOutputStream doing a network flush) will pin whatever runtime worker is driving the write task. If that handle is a shared smol executor pool, this can stall unrelated tasks. The doc comment acknowledges writes "run inline on the runtime thread," but it's worth confirming this is a dedicated thread, or moving the upcalls to spawn_blocking for symmetry with the read path.

3. JNI global-ref drop needs a JVM attachment
JavaFileEntry.readable / JavaWrite.writable hold Global<JObject> refs that are dropped when NativeDataSource/NativeWriter is freed. Dropping a global ref requires an attached thread. NativeDataSource_free runs on a Java thread (fine), but confirm the same holds if a JavaReadable/JavaWrite Arc is last-dropped on a native runtime worker (e.g. a lingering scan task) — otherwise the ref leaks or aborts depending on the jni crate's Drop impl. Worth a test that drops the data source while a scan future is still alive.

Performance

4. Read path does a double allocation + copy — io/read_at.rs:146-160
Each read_at allocates both a native aligned buffer (DefaultHostAllocator.allocate) and a fresh Java byte[], has Java fill the byte[], then copies it into the native buffer via get_region. Given this crate's emphasis on zero-copy, consider handing Java a direct ByteBuffer wrapping the native buffer so readFully writes straight into it — eliminating the throwaway byte[] and the copy. That's an interface change (readFully(long, ByteBuffer, int)), so reasonable to defer, but worth capturing.

Minor / style

• read_at.rs:146 — the native buffer is allocated before the upcall, so it's wasted work when readFully throws. Minor; could allocate after a successful upcall if you switch away from get_region.
• The author's own pending review notes (data_source.rs: "do not keep the original string"; read_at.rs: "you're keyed by this already", "remove this"; errors.rs: "import this") appear to already flag redundant state — the NativeReadable.name()/length() interface methods overlap with the names/lengths arrays passed to openFiles. Consolidating to a single source of truth would remove the duplication.

Tests

NativeIOBridgeTest.java (+320) gives good coverage of the happy path. Consider adding: an out-of-bounds/overflow read, a readFully that throws IOException (to confirm the exception surfaces as a VortexError and not a swallowed default), and a concurrent-read stress test that exercises the UpcallLimiter semaphore.

Overall this looks solid and close to mergeable — the overflow check (#1) is the only item I'd consider blocking; the rest are hardening and follow-ups.
· branch rk/icebergio

[0ax1]

trusting in the tests here

[robert3005]

I think we need to fix the blocking and double copy. The java plumbing makes it easy to miss

[github-actions]

Benchmarks: Vortex queries

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +2.4%
Engines: DataFusion No clear signal (+9.3%, medium confidence) · DuckDB No clear signal (-4.0%, low confidence)
Vortex (geomean): 0.957x ➖
Parquet (geomean): 0.946x ➖
Shifts: Parquet (control) -5.4% · Median polish -3.7%

How to read Verdict and Engines

• Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
• Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
• Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

---

datafusion / vortex-file-compressed (0.995x ➖, 0↑ 0↓)

name	PR f8a4363 (ns)	base 01e5eb0 (ns)	ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed	9947687	9772504	1.02
vortex_q01/datafusion:vortex-file-compressed	6299178	6477516	0.97

datafusion / parquet (0.910x ➖, 1↑ 0↓)

name	PR f8a4363 (ns)	base 01e5eb0 (ns)	ratio (PR/base)
vortex_q00/datafusion:parquet	19623233	21257508	0.92
vortex_q01/datafusion:parquet 🚀	4420342	4923363	0.90

duckdb / vortex-file-compressed (0.944x ➖, 0↑ 0↓)

name	PR f8a4363 (ns)	base 01e5eb0 (ns)	ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed	10238881	10663593	0.96
vortex_q01/duckdb:vortex-file-compressed	5955178	6422596	0.93

duckdb / parquet (0.983x ➖, 0↑ 0↓)

name	PR f8a4363 (ns)	base 01e5eb0 (ns)	ratio (PR/base)
vortex_q00/duckdb:parquet	23279624	23624422	0.99
vortex_q01/duckdb:parquet	9367187	9545825	0.98

---

No file size changes detected.

[github-actions]

Polar Signals Profiling Results

Latest Run

Status	Commit	Job	Attempt	Link
🟢 Done	f8a4363		1	Explore Profiling Data

Previous Runs (1)

Status	Commit	Job	Attempt	Link
🟢 Done	b63c6ae		1	Explore Profiling Data

---

Powered by Polar Signals Cloud