Conversation
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Martin Prammer <martin@spiraldb.com>
Merging this PR will degrade performance by 6.02%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decode_primitives[f32, (1000, 512)] |
42 µs | 59.6 µs | -29.52% |
| ❌ | WallTime | dbp_assemble_kernel_avx512[(I128, 1024)] |
463 ns | 550 ns | -15.82% |
| ❌ | WallTime | dbp_assemble_kernel_avx2[(I128, 1024)] |
464 ns | 535 ns | -13.27% |
| ❌ | Simulation | take_fsl_u32_random[128, 100] |
208.3 µs | 237.2 µs | -12.18% |
| ❌ | WallTime | dbp_assemble_kernel_narrow_msp_avx2[(I128, 1024)] |
629 ns | 708 ns | -11.16% |
| ❌ | Simulation | take_fsl_f16_random[256, 100] |
204.9 µs | 230.5 µs | -11.11% |
| ⚡ | Simulation | take_fsl_f16_random[16, 100] |
179.5 µs | 119.4 µs | +50.28% |
| ⚡ | Simulation | take_fsl_nullable_random[16, 100] |
188 µs | 165.7 µs | +13.43% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing mprammer:mp/ffi-reader-at-expressions (be7f8d7) with develop (a542cbd)
Footnotes
-
293 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
@claude review |
|
Claude review automation is disabled for fork pull requests. Why:
If maintainers want Claude to implement a change, restate the task on an issue and use the issue-driven Claude workflow instead. |
Addresses review feedback on the C FFI callback reader. The read callback now returns the number of bytes written rather than a status code, so a host that reports success without filling the buffer is rejected instead of handing uninitialized memory to the scan. The Java bindings already guard this via `remaining()`; the previous C signature made it undetectable. A source was owned by the segment source's spawned read driver, which the FFI's current-thread runtime never polled again once a call returned, so `release` ran at an unpredictable later point or not at all. Hosts freeing their context after `vx_data_source_free` could therefore hit a use-after-free. `CurrentThreadRuntime::drain` runs queued tasks to a stop, and the FFI drains after dropping a data source and after a failed open, so `release` now runs before the call that drops the source returns. Callback concurrency is additionally capped process-wide rather than only per source, matching how the Java bindings bound upcalls across a filesystem, and a name view of NULL with a non-zero length is rejected rather than silently treated as anonymous. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Martin Prammer <martin@spiraldb.com>
| pub name: vx_view, | ||
| /// Required. Writes "length" bytes at "offset" into "dst" and returns the | ||
| /// count written; a short count or a negative value fails the read. | ||
| pub read_at: Option< |
This branch has not been deployed
2 participants
Lets a host serve file bytes through its own I/O stack instead of reading the whole file into memory, so a scan fetches only the segments it needs. The C form of
dev.vortex.io.NativeReadable.Changes
vx_readatcarries the host'sread_atandreleasecallbacks, the source length and an optional name. Reads run on the blocking pool underCoalesceConfig::object_storage(), matchingJavaReadable. Ownership is worth a close read: a rejected descriptor never callsreleaseand leaves the context with the caller; an accepted one callsreleaseeven if opening the file then fails.concurrencycaps reads per source, not across open files, so opening many files multiplies it — no equivalent of the JNI shared limiter yet.row_idxandpackcome with it because the reader alone doesn't get Iceberg onto Vortex: positional deletes and row lineage are defined on physical row positions, which must be projected alongside the data to survive predicate pushdown.packassembles a projection rather than trimming one, which is also how nested struct fields get pruned.Tests read a file served entirely through the callbacks, cover both
releasepaths and a failing callback, and check thatrow_idxunder a pushed-down filter returns surviving rows at their original positions.API Changes
Added:
vx_data_source_new_readat(session, reader, err)— data source over caller-supplied read callbacks.vx_expression_row_idx()— row position within the scanned file; only valid inside a scan.vx_expression_pack(names, expressions, len, nullable)— struct-valued expression from named children.🤖 Generated with Claude Code