Keep GPU FSST decompression on device - #9414
Draft
joseph-isaacs wants to merge 23 commits into
Draft
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>
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>
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>
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>
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>
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>
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>
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>
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>
…ess-benchmarks-4mmn93 Signed-off-by: Claude <noreply@anthropic.com>
The verification gate ran before the timed pass, so a single unsupported dataset meant the run produced no numbers at all. Two changes: - the per-dataset survey now applies to any GPU run, not just a verifying one, and the timing tables render before the failure summary, so datasets that do decode still publish their numbers; - the workflow runs the benchmark before the gate and fails the job at the end on either a failed verification or a failed benchmark. Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
The per-dataset grep captures only the first line of each error, so a Python traceback from the cuDF script or a Rust backtrace never reached the comment. Attach the tail of the raw output in a collapsed block on failure, and match the summary line's current wording. Signed-off-by: Claude <noreply@anthropic.com>
fix(cuda): copy validity back to the host in `into_host` `CanonicalCudaExt::into_host` migrated a canonical array's values buffer but passed its validity through untouched, so a nullable array came back to the host half-migrated and the first host read of the mask panicked with "unwrap_host called for Device allocation" — via `Validity::execute_mask` -> `BoolArray::into_bit_buffer`. Non-nullable arrays were unaffected, which is why it only showed up on the Public BI tables. The `Bool` arm already carried a TODO for exactly this. Do not compare Parquet DATE columns across representations pyarrow materialises a DATE column as `datetime.date` objects and cuDF as `datetime64[s]`. The values agree, but `check_dtype=False` does not bridge object-vs-datetime64, so the comparison reported every row as different and failed both TPC-H datasets. Coerce both sides to datetime64 first. Read the Vortex GPU file through the page cache by default cuDF takes an untimed warm-up read, so its timed read is served from the page cache, while the Vortex reader used `O_DIRECT` on every iteration and paid real disk reads each time. That compared a read of the disk against a read of RAM. Direct IO is now off by default and available behind `--gpu-direct-io` for measuring storage bandwidth, which is not a decode comparison. Signed-off-by: Claude <noreply@anthropic.com>
The GPU ratio says which of two GPU readers is faster, not whether either beats the CPU decoders. Run the same binary over the same datasets with the CPU path on the same machine and publish it alongside, so the GPU numbers can be read against something. Also capture the benchmark's exit status rather than letting `shell: bash`'s -e skip the `cat`, which kept the timing tables out of the job log and left them only in the PR comment. Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
joseph-isaacs
added
the
action/bench-gpu-compress
github-actions
Bot
removed
the
action/bench-gpu-compress
Merging this PR will degrade performance by 14.59%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | cuda/alp_f64/10%[100M] |
6.9 ms | 8.4 ms | -17.63% |
| ❌ | WallTime | cuda/alp_f32/0%[100M] |
2.5 ms | 2.9 ms | -16.55% |
| ❌ | WallTime | cuda/alp_f64/1%[100M] |
6.6 ms | 7.7 ms | -14.06% |
| ❌ | WallTime | cuda/alp_f32/1%[100M] |
4.8 ms | 5.6 ms | -13.99% |
| ❌ | WallTime | cuda/alp_f32/10%[100M] |
4.5 ms | 5.1 ms | -12.92% |
| ❌ | Simulation | cold_misaligned[(64, 256)] |
4.4 ms | 5 ms | -12.24% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing joe/gpu-decompress-on-device (5b7e3a3) with develop (b363fb7)
Footnotes
-
46 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. ↩
Contributor
GPU decompression verificationVerification failed. Per-dataset results: Full error detail |
Contributor
GPU CompressionCPU baseline (same machine, same datasets) |
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
joseph-isaacs
added
the
action/bench-gpu-compress
github-actions
Bot
removed
the
action/bench-gpu-compress
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
Signed-off-by: Joe Isaacs <2413449+joseph-isaacs@users.noreply.github.com>
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
2 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.
Supersedes #9147 with a fairer GPU decompression comparison and device-side FSST output construction.
What changed
--formats vortexto isolate Vortex-only GPU diagnostics; the fair default remainsparquet,vortexWhy
The earlier benchmark gave Vortex hundreds of small input chunks while Parquet used large row groups, and FSST copied every decoded string length to the CPU to build output offsets. That made launch overhead and a bulk device-to-host synchronization dominate the Vortex number.
Matching physical partitions makes the comparison equivalent. Keeping lengths, offsets, output bytes, and BinaryViews on the GPU removes the bulk CPU round trip. Trusted exact length statistics also remove the two remaining scalar D2H synchronizations from each FSST decode.
Results
TPC-H lineitem, six equal physical partitions and all 96 field decodes:
Validation
cargo test -p vortex-cuda fsst --lib— 22 passedcargo test -p vortex-cuda test_cuda_datetimeparts_unsigned_components --lib— passedcargo check -p vortex-cuda -p vortex-btrblockscargo clippy -p vortex-cuda --all-targets --all-features -- -D warningscargo build -p compress-bench --release --features cuda,unstable_encodingscargo +nightly fmt --allclang-format --dry-run --Werror --style=file vortex-cuda/kernels/src/arrow_offsets.cu vortex-cuda/kernels/src/date_time_parts.cugit diff --check--gpu-verify— all 96 fields matched the CPU decoder--gpu-verify— all 110 fields matched the CPU decoder--gpu-verify— all 76 fields matched the CPU decoder