Compare Vortex GPU decompression against a cuDF Parquet read - #9147
Open
joseph-isaacs wants to merge 24 commits into
Open
Compare Vortex GPU decompression against a cuDF Parquet read#9147joseph-isaacs wants to merge 24 commits into
joseph-isaacs wants to merge 24 commits into
Conversation
The GPU compression benchmark only measured Vortex, and only on a single dataset, so it could not say anything about how Vortex GPU decompression compares to Parquet, nor about encodings beyond FSST strings. Parquet compresses each page body independently, which is exactly the batch shape nvCOMP's device decompressors take and how cuDF's Parquet reader gets pages off the CPU. This adds a Parquet backend built on that: column chunks are staged on the device through the same pinned, direct-I/O reader the Vortex backend uses, then every page in a row group is decompressed in one batched nvCOMP launch. - vortex-nvcomp: bind the batched Snappy decompression entrypoints and the per-algorithm alignment queries, and share `DecompressBackend` between the Snappy and Zstd wrappers. - compress-bench: locate compressed page bodies by walking the per-page Thrift headers (`parquet::format::PageHeader` is deprecated and `parquet`'s own parser is crate-private), and write files with GPU-friendly settings: v1 pages, dictionary encoding, 1 MiB pages, Snappy by default. - Run both Vortex and Parquet under `--gpu-decompress`, and expand the GPU dataset set from one to nine so ALP, bit-packed, run-end, date/time-parts and null-heavy columns are covered alongside FSST strings. - Add `--gpu-verify`, which compares every GPU-decompressed page against the host codec and every GPU-decoded Vortex field against the CPU decode, and run it as a CI step before the timed benchmark. Independently of that flag, nvCOMP's per-page status and size arrays are checked on every iteration. Page decoding is not part of the Parquet measurement, so its numbers are an upper bound on a full GPU Parquet reader; the README states this. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
A CUDA scan hands back arrays whose buffers live in device memory, so decoding those same arrays through the host Arrow path panics rather than producing a CPU reference. Read the file a second time through the ordinary host reader and compare the two scans batch by batch instead. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
Two changes to the Vortex GPU verification, after CI reported a `fastlanes.for` mismatch with no detail: - Read the CPU reference from a copy of the file. The session segment cache is keyed by URI and the CUDA reader deliberately bypasses it because its buffers are device-resident, so pointing both scans at one URI risks them sharing entries. - Synchronize the stream before copying a decoded field back, and report the Arrow types, lengths, null counts and the first differing row when the two decodes disagree. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
Merging this PR will not alter performance
|
The bit-unpack kernel writes patch values straight into the output while the lane decoder adds the frame of reference to every unpacked value. Bit-packing exceptions are stored in the same reference-relative domain as the packed values, so under `FoR(BitPacked)` every patched position came out short by exactly the reference. The existing kernel tests could not catch this: they exercise `BitPacked` directly, where the reference is zero. The new `FoRExecutor` case bit-packs to 8 bits with values that overflow into patches and a non-zero reference. Found by the compression benchmark's new `--gpu-verify` pass, which reported a `fastlanes.for` field decoding row 8038 as 131072 where the CPU produced 393061 — a difference of exactly the 261989 reference. Also thread the dataset name through compress-bench failures, so a benchmark error says which dataset it came from. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
A verification run stopped at the first dataset that failed, so finding the GPU-clean set took one CI cycle per dataset. Run every dataset instead, recording failures and reporting them together at the end, then exit non-zero. Missing CUDA kernel support surfaces as a panic rather than an error, so the survey catches those too. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
The per-dataset verification verdicts were only visible by digging through a multi-thousand-line job log. Capture the verification output, publish the per-dataset results to the step summary and a PR comment, and keep failing the job through a separate gate step. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
Contributor
GPU decompression verificationVerification failed. Per-dataset results: Full error detail |
The Public BI datasets build their Parquet fixture through the DuckDB CLI, as in bench-pr.yml. The GPU job never installed it, so all six failed with ENOENT before reaching the GPU at all. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
The nvCOMP backend only ran the codec stage on the device: page decoding stayed on the CPU and was excluded from the measurement, so the Parquet figure was an upper bound and the comparison against Vortex was not like-for-like. cuDF's `read_parquet` does the whole read on the device — page header decode, decompression, dictionary/RLE/plain decoding and column assembly — which is the same amount of work the Vortex backend does when it decodes to canonical arrays. It is reached through the prebuilt `cudf-cu12` wheel, so it stays a runtime dependency and never enters the Rust build. Timing is taken inside scripts/cudf-parquet-read.py, so interpreter start, `import cudf` and CUDA context creation are excluded; a warm-up read runs first. `--gpu-verify` now compares the cuDF frame against a CPU Parquet read. This removes the page scanner, the batched nvCOMP launch path and the nvCOMP Snappy bindings, all of which existed only to serve the codec-stage backend. What remains of the Parquet side is the GPU-friendly writer settings, now in gpu_writer.rs. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
force-pushed
the
claude/gpu-decompress-benchmarks-4mmn93
branch
from
August 13, 2026 15:04
c93d5cc to
64d481c
Compare
joseph-isaacs
added
action/benchmark-gpu-compress
and removed
action/benchmark-gpu-compress
labels
Claude
The reference side of the Vortex verification was executing through the CUDA context: the host scan's batches and both Arrow conversions were handed `cuda_ctx.execution_ctx()`. A CUDA context allocates its outputs in device memory, so the Arrow conversion then read a device buffer from the host and panicked with "unwrap_host called for Device allocation" on the string-heavy Public BI datasets, where canonicalisation goes through the buffer directly. Signed-off-by: Claude <noreply@anthropic.com>
joseph-isaacs
added
the
action/bench-gpu-compress
This was referenced Aug 17, 2026
Contributor
|
Can we make the PR description a bit more compact? 🙂 What do we compare correctness or performance? |
0ax1
reviewed
Aug 17, 2026
| with: | ||
| repo-token: ${{ secrets.GITHUB_TOKEN }} | ||
| enable-sccache: "true" | ||
| - name: Install DuckDB |
Contributor
There was a problem hiding this comment.
We should centralize this somewhere. I think we have the CI logic for installing DuckDB duplicated like 5 times.
0ax1
reviewed
Aug 17, 2026
| //! the like-for-like opponent for the Vortex GPU backend, which likewise decodes all the way | ||
| //! to canonical arrays on device. | ||
| //! | ||
| //! cuDF is reached through its prebuilt `cudf-cu12` wheel rather than by linking libcudf, so |
Contributor
There was a problem hiding this comment.
What does Oh right, we invoke the Parquet GPU backed via Python and spawn a subprocess. Shall we make the more clear here?wheel mean here?
| /// Repo-relative path of the script that performs and times the cuDF read. | ||
| const CUDF_SCRIPT: &str = "scripts/cudf-parquet-read.py"; | ||
|
|
||
| /// Parquet compressor whose decompression measurement is a full cuDF GPU read. |
Contributor
There was a problem hiding this comment.
I assume full means the complete file, unfiltered and all columns.
| /// Parquet compressor whose decompression measurement is a full cuDF GPU read. | ||
| pub struct GpuParquetCompressor { | ||
| codec: GpuCodec, | ||
| verify: bool, |
Contributor
There was a problem hiding this comment.
verify deserves a comment. Does Parquet GPU have a verification for the returned results built in?
| columns: u64, | ||
| } | ||
|
|
||
| impl GpuParquetCompressor { |
Contributor
There was a problem hiding this comment.
Are we doing a single iteration or multiple actually? And if one, is that sufficient?
| let mut fields_checked = 0usize; | ||
| let mut batch_index = 0usize; | ||
| loop { | ||
| let (gpu_batch, host_batch) = (gpu_batches.next().await, host_batches.next().await); |
Contributor
There was a problem hiding this comment.
Having the break condition in the header of a while loop would be more explicit.
| //! Parquet writer settings for the GPU benchmark. | ||
| //! | ||
| //! The GPU backend rewrites each dataset before reading it back with cuDF, so the file it | ||
| //! reads is written the way a GPU reader wants it rather than the way the CPU suite writes it. |
Contributor
There was a problem hiding this comment.
wants it is a little lose in wording
| #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, ValueEnum)] | ||
| pub enum GpuCodec { | ||
| /// The Parquet default, and the codec with the highest device-side throughput. | ||
| #[default] |
Contributor
There was a problem hiding this comment.
Oh does Parquet only do Snappy and Zstd for GPU?
| /// Get a compressor for the given format. | ||
| fn get_compressor(format: Format, gpu_decompress: bool) -> Box<dyn Compressor> { | ||
| if gpu_decompress { | ||
| fn get_compressor(format: Format, gpu: Option<GpuOptions>) -> Box<dyn Compressor> { |
Contributor
There was a problem hiding this comment.
API is a little unclear here, with options being optional. 😅 gpu: Option<GpuOptions>). Could we pass GpuOptions and name the parameter options?
| Format::Parquet => Box::new(GpuParquetCompressor::new(gpu.codec, gpu.verify)), | ||
| _ => unimplemented!("GPU compress bench not implemented for {format}"), | ||
| }; | ||
| #[cfg(not(feature = "cuda"))] |
Contributor
There was a problem hiding this comment.
Yeah I don't think the fact that the code is either compiled on Linux or macOS should not be represented by the Option.
Contributor
|
codex review: Findings
Lower-priority issues
|
Centralize the DuckDB CLI install as a `setup-duckdb` composite action and use it from all four workflows that had the download inlined. Behaviour changes, each from a review thread: - `GpuParquetCompressor::compress` now refuses like the Vortex GPU backend does. It timed the host Parquet writer, which is not a device measurement, and GPU mode only runs the decompress op so the number was never rendered. - `--gpu-verify` no longer returns its own elapsed time as the decompress measurement. That bundled a file copy, a second host scan and every Arrow conversion into a number the timing table published as a decode time. Verification is now a precondition and the ordinary timed scan runs after it, so a verifying run reports a comparable number. - The cuDF script is asked for several timed reads per invocation instead of the default one, so the reported minimum is over repeated reads rather than a single sample. - `--gpu-direct-io` on a non-Linux target is now an error instead of a silent no-op, since the flag changes what the measurement means. Readability, also from review: - Build the cuDF `Command` in one expression, and move the "is cudf-cu12 installed" hint onto the non-zero-exit arm, where a failed `import cudf` actually surfaces. - Give the verification loop its exit condition in the header via `next_batch_pair`, which keeps a batch-count mismatch an error rather than a stopping condition. - Rename the `Option<GpuOptions>` parameters to `options`/`gpu_options`. Comment fixes: spell out that the Parquet backend drives cuDF through a `python3` subprocess rather than a linked library; say that "whole-file" means every row and column with no pushdown; document the `verify` field and that cuDF has no built-in verification; note that the two-codec `GpuCodec` set is a deliberate choice, not a format limit; and reword the writer module header. README updated to match. Checks: `cargo clippy -p compress-bench --all-targets -- -D warnings` and the same with `--features cuda,unstable_encodings` both pass; `cargo test -p compress-bench --lib` passes; `yamllint --strict -c .yamllint.yaml` passes on the five changed YAML files. Not run: the CUDA tests and the GPU benchmark itself, which need a GPU. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FnRK96p8jiB9NdSKxU2iy
`Option<GpuOptions>` conflated two questions: which suite this run measures, and how the
GPU is configured. GPU mode is not a setting on the CPU suite — it picks a different
dataset list, restricts the ops to decompression, swaps both compressors and labels its
ratio differently — so `None` was carrying the weight of "run the host suite".
Replace it with `BenchMode { Cpu, Gpu(GpuOptions) }`. The `is_some()` checks become
`is_gpu()`, and the ratio dispatch and compressor selection match on the variant, so no
site has to know that an absent config means the CPU path.
Checks: `cargo clippy -p compress-bench --all-targets -- -D warnings` and the same with
`--features cuda,unstable_encodings` both pass; `cargo test -p compress-bench --lib`
passes.
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016FnRK96p8jiB9NdSKxU2iy
joseph-isaacs
marked this pull request as ready for review
August 17, 2026 12:48
The list carried nine datasets while its own comment said to add one "only after its CUDA-compatible compression and decompression kernels have been verified end to end". Five had not been: the last published verification matrix showed `taxi` and `Arade` failing on `Unsupported ptype u16`, `Euro2016` and `HashTags` on a missing `vortex.masked` kernel, and `CMSprovider` on `expected host buffer`. Since a failing dataset makes the run exit non-zero, the benchmark job could not go green. Keep the four that verified — both TPC-H `l_comment` variants, `Bimbo` and `Food` — and record the other five next to the list with the `vortex-cuda` gap each is waiting on, so closing a gap has an obvious place to re-add its dataset. The per-dataset survey and failure summary stay, so a listed dataset that regresses is still reported rather than aborting the run. Checks: `cargo clippy -p compress-bench --all-targets -- -D warnings` and the same with `--features cuda,unstable_encodings` both pass. The GPU benchmark itself needs a GPU and was not run here, so the four remaining datasets should be re-confirmed with `--gpu-verify` on a GPU runner. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FnRK96p8jiB9NdSKxU2iy
`uvx ruff format --check .` failed the Python (lint) job on the `--iterations` argument: the call was wrapped across three lines, but at 91 characters it fits inside the repository's 120-character limit, so ruff collapses it to one line. Ran `uvx ruff format` rather than hand-editing so the result matches CI byte for byte. `uvx ruff format --check .` and `uvx ruff check .` both pass locally now. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FnRK96p8jiB9NdSKxU2iy
joseph-isaacs
force-pushed
the
claude/gpu-decompress-benchmarks-4mmn93
branch
from
August 17, 2026 18:36
085a592 to
29ccc8b
Compare
Two whole functions in `canonical.rs` carried `#[allow(clippy::disallowed_methods)]`, which silences the `legacy_session` lint across every line in them — including code added later that nobody chose to waive. Replace both with `#[expect(clippy::disallowed_methods, reason = ...)]` on the three `legacy_session().create_execution_ctx()` bindings themselves, matching the form already used 177 times elsewhere in the tree. `expect` also fails the build if the call is ever threaded a real session, so the waiver cannot outlive the problem. Checks: `cargo clippy -p vortex-cuda --all-targets -- -D warnings` passes, which also proves each of the three expectations is fulfilled at its narrowed scope. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FnRK96p8jiB9NdSKxU2iy
`verify_against_host_scan` held stream setup, batch pairing, field pairing, the CUDA decode and the comparison in one body. Move everything between the two `ensure!` guards into `verify_batch`, which returns the number of fields it checked, so the caller is left with the scan wiring and a running total. Drop the two `collect::<Vec<_>>()` calls while moving it: `iter_unmasked_fields` is already an `ExactSizeIterator`, so the count check reads its `len()` directly and the fields zip as iterators instead of being cloned into two vectors per batch. Only the GPU side still clones, because `execute_cuda` takes `self`. Checks: `cargo clippy -p compress-bench --all-targets --features cuda,unstable_encodings -- -D warnings` passes. The verification path itself needs a GPU and was not run here. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FnRK96p8jiB9NdSKxU2iy
0ax1
approved these changes
Aug 18, 2026
joseph-isaacs
force-pushed
the
claude/gpu-decompress-benchmarks-4mmn93
branch
from
August 18, 2026 09:48
3f3cff6 to
537e30b
Compare
joseph-isaacs
added
the
action/bench-gpu-compress
github-actions
Bot
removed
the
action/bench-gpu-compress
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
joseph-isaacs
force-pushed
the
claude/gpu-decompress-benchmarks-4mmn93
branch
from
August 18, 2026 12:07
398074e to
537e30b
Compare
joseph-isaacs
added
the
action/bench-gpu-compress
Claude
github-actions
Bot
removed
the
action/bench-gpu-compress
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The GPU decompression benchmark measured only Vortex, so the
--gpu-decompressnumbers hadnothing to compare against — a device-side decode rate is only meaningful next to another
format decoded on the same device.
The comparison point is cuDF's
read_parquet, which doesthe whole Parquet read on the GPU: page header decode, codec decompression, dictionary/RLE/plain
decoding and column assembly. Both sides therefore decode all the way to device-resident arrays,
which is what makes the ratio a like-for-like number rather than a comparison against a host read.
Adding a second decoder also gave the Vortex CUDA path a reference to be checked against, and
that cross-check found two real correctness bugs in
vortex-cuda(see below).What changes are included in this PR?
GPU Parquet backend.
benchmarks/compress-bench/src/gpu_parquet.rsrewrites each datasetwith GPU-friendly writer settings and times a cuDF read of it. cuDF is reached through its
prebuilt
cudf-cu12manylinux wheel and driven byscripts/cudf-parquet-read.py, so it is aruntime dependency of the benchmark and never enters the Rust build. Timing is taken inside the
script after a warm-up read, so interpreter start,
import cudfand CUDA context creation areexcluded.
GPU-friendly Parquet writer settings (
src/gpu_writer.rs): v1 pages, Snappy (default) orZstd, dictionary enabled, 1 MiB data pages with a 1,000,000-row limit, chunk-level statistics.
The rationale for each is tabulated in the README.
Like-for-like I/O. cuDF takes an untimed warm-up read, so its timed read is served from the
page cache. The Vortex reader therefore no longer uses
O_DIRECTby default — doing so compareda Vortex read of the disk against a cuDF read of RAM on every iteration.
--gpu-direct-iorestores it for measuring storage bandwidth, which is a different question and not a decode
comparison.
Correctness.
--gpu-verifycross-checks both backends against the CPU decoders inline: thecuDF frame against a CPU Parquet read, and each GPU-decoded Vortex field against a separate
host-only scan of a copy of the file, compared through Arrow with a pinned target type. A
mismatch reports the differing types, lengths and null counts plus the first differing row,
found by binary search.
Two
vortex-cudabugs found by the cross-check(exception) values without adding the reference, and patches are stored reference-relative —
so any patched value under
FoR(BitPacked)decoded on the GPU came back short by exactly thereference. Fixed in
vortex-cuda/src/bit_unpack_gen.rs, kernels regenerated, with an rstestregression case in
vortex-cuda/src/kernel/encodings/for_.rscovering u32/u64 patches atlane, block and cross-block boundaries.
into_hostleft validity on the device.CanonicalCudaExt::into_hostmigrated acanonical array's values buffer but passed its
Validitythrough untouched, so a nullablearray came back half-migrated and the first host read of the mask panicked in
BufferHandle::unwrap_host(viaValidity::execute_mask→BoolArray::into_bit_buffer).Non-nullable arrays were unaffected, which is why it only surfaced on the nullable Public BI
tables. The
Boolarm already carried a TODO for exactly this.Benchmark coverage.
--gpu-decompressnow runs nine datasets (TPC-Hl_commentcanonicaland chunked, taxi, and the Arade/Bimbo/CMSprovider/Euro2016/Food/HashTags Public BI tables)
rather than one, and reports on every dataset instead of stopping at the first failure — one run
shows the whole matrix. The timing tables render before the failure summary, so datasets that do
decode still publish numbers when another dataset cannot.
CI.
pr-bench-gpu-compress.ymlinstalls the DuckDB CLI (needed to build the Public BIfixtures, as in
pr-bench-compress.yml) and the cuDF wheel, runs a verification pass and thetimed pass, publishes both to the PR — attaching full tracebacks and backtraces on failure —
and fails the job at the end if either failed.
Known gaps
Two CUDA encoding gaps remain, both in
vortex-cudarather than in the benchmark, and bothoutside the scope of this PR:
taxiandArade:Unsupported ptype u16. The CUDAdate_time_partskernel dispatches withmatch_each_signed_integer_ptype!while the CPU canonicaliser usesmatch_each_integer_ptype!.Widening the fused kernel's dispatch turns 4³ = 64 PTX instantiations into 8³ = 512, so the fix
is not free.
Euro2016andHashTags:No CUDA kernel for encoding vortex.masked.What APIs are changed? Are there any user-facing changes?
One library behaviour change:
CanonicalCudaExt::into_hostnow migrates validity as well asvalues, so a nullable canonical array copied back from the device is fully host-resident. That
is a bug fix — the previous result panicked on first use.
Everything else is confined to the
compress-benchbinary. New benchmark CLI flags:--gpu-parquet-codec,--gpu-verifyand--gpu-direct-io. Running--gpu-decompressnowadditionally requires the
cudf-cu12wheel onPATH;benchmarks/compress-bench/README.mddocuments the install and the remaining transfer-path asymmetry (the Vortex reader uses pinned
buffers; cuDF does its own host read and host-to-device copy).