Repository navigation
fix(cuda): copy encoded device-resident children to the host in into_host - #10916
Merged
Merged
Conversation
…host CanonicalCudaExt::into_host executed a child that CUDA execution left encoded on the host, with its buffers still on the device. A run-end encoded validity mask is such a child, and so is a nullable dictionary's validity, a lazy take of the values' validity through the device-resident codes. Both panicked on a host-buffer read: compress-bench --gpu-verify fails this way on CMSprovider and Euro2016 with the current CUDA preset. The child's buffers are now copied to the host before it is executed there. Children that are already canonical take the existing path, the copies run concurrently, and an array's buffers are replaced only when one of them moves. Adds tests that copy to the host a primitive array whose validity is a run-end or sparse encoded mask on the device, and string and primitive dictionaries with nullable values and device-resident codes. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Tanish Gudise <[email protected]>
This was referenced Oct 28, 2026
Merging this PR will not alter performance1 warning
Comparing |
0ax1
approved these changes
Nov 10, 2026
0ax1
left a comment
Contributor
There was a problem hiding this comment.
Change looks good! Thanks again for your contribution!
0ax1
added a commit
that referenced
this pull request
Nov 10, 2026
…eset (#10917) ## Summary - Tracking Issue: #10914 - Depends on #10915 and #10916; this branch contains both. The diff of its own is one line plus a test. `CompressionMode::Cuda` excludes `StringDictScheme`, but `execute_dict_varbinview` already decodes string dictionaries on the GPU. Keeping the scheme makes GPU-mode files 16.0% smaller across compress-bench's 17-dataset GPU suite (3,351.3 → 2,813.9 MB). Per-dataset gains: CMSprovider 45.7%, Food 52.9%, HashTags 33.2%, both TPC-H l_comment variants 12.2%, Euro2016 10.7%. The issue has the full table. ## Changes - `vortex-btrblocks/src/builder.rs`: drops `StringDictScheme` from the Cuda exclusions and updates the comment. A new test checks that the preset keeps string dictionaries but not binary ones. This is the only change to CPU-compressor code. Evidence with this branch (A40, CUDA 13.0): - `--gpu-verify` passes on all 17 datasets. develop plus only this line fails 5: taxi, CMSprovider, Euro2016, Food, HashTags. - The CPU-fallback census is identical to develop's: 59 constant-array fallbacks, none from dictionaries. - The dictionary gather takes at most 6.1% of decode time and reaches 62-85% of the measured 563 GB/s. - `vortex-cuda` lib suite on this branch: 723 passed, 0 failed. - `compress_gpu` fuzz (CI flags, ASAN): 100,000 runs in 256 s, 0 crashes. ## Benchmarks Setup: - `compress-bench --gpu-decompress --iterations 5`, a fresh process per run, `taskset -c 2-7`, 3 ABBA rounds. - Speedup = develop time / branch time. - The same-design A/A floor is 0.14 in |log|, so a "faster" claim needs a 95% CI above 1 and a speedup above 1.32×. | Dataset | File MB, develop → branch | Speedup [95% CI] | | |---|---|---:|---| | Food | 84.7 → 39.9 | 1.55× [1.41, 1.71] | faster | | CMSprovider | 771.0 → 418.5 | 1.37× [0.66, 2.86] | within noise | | Euro2016 | 163.3 → 145.9 | 1.46× [0.92, 2.32] | within noise | | TPC-H l_comment chunked | 169.6 → 148.8 | 1.36× [0.55, 3.34] | within noise | | taxi | 52.0 → 51.5 | 1.28× [0.32, 5.11] | within noise | | HashTags | 235.2 → 157.0 | 1.22× [0.89, 1.69] | within noise | | TPC-H l_comment canonical | 169.6 → 148.8 | 1.18× [0.71, 1.96] | within noise | | Arade | 144.2 → 141.7 | 1.05× [0.42, 2.68] | within noise | The nine datasets whose files don't change got one AB/BA round each. Eight came out at 1.01-1.18×, and one (wide table cols=1000 chunks=50) at 0.91×, inside the A/A floor. The measurements are from an A40 (GA102); the repo's GPU benchmarks run on A10G. ## API changes `only_cuda_compatible()`, and therefore `cuda_write_strategy`, can now emit string dictionaries. - An `ArrowDeviceArrayStream` with the default `DictionaryExport::Preserve` rejects a batch whose Arrow schema differs from the first. - With this change, 6 of CMSprovider's 14 string columns mix dictionary and constant chunks. - Today's Cuda files already do this in numeric columns: 7 of taxi's 18 numeric columns and 5 of CMSprovider's 12. - Streams with `DictionaryExport::Decode`, which the FFI scan uses, are unaffected. Overlap: #10425 adds a line next to the removed one, so whichever merges second needs a one-line rebase. #9762 changes dictionary selection, so the sizes are as of a59294e. ## AI assistance Developed with Claude Code (Claude Opus 5.5), which wrote most of the code, tests and this description under my direction. I reviewed the diff and the measurements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tanish Gudise <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: Alexander Droste <[email protected]>
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
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.
Summary
compress-bench --gpu-decompress --gpu-verifypanics withexpected host bufferon develop for CMSprovider and Euro2016. A run-end encoded validity mask stays encoded on the device afterexecute_cuda.CanonicalCudaExt::into_hostthen executes it on the host while its buffers are still on the device (validity_into_host→child_into_host→RunEnd::execute→runend_decode_bools→BufferHandle::as_host).A nullable dictionary's validity, a lazy
takethrough device-resident codes, fails the same way.Changes
canonical.rs:child_into_hostcopies every device buffer of a child that isn't canonical to the host before executing it there.into_hostwith the expected result:Evidence (A40, CUDA 13.0):
expected host buffer(run-end mask),as_slice must be called on host buffer(sparse mask, both dictionaries)--gpu-verifyon CMSprovider and Euro2016, today's presetvortex-cudalib suiteEverything in the table ran on this PR alone. With all three PRs,
--gpu-verifypasses on all 17 GPU-suite datasets.AI assistance
Developed with Claude Code (Claude Opus 5.5), which wrote most of the code, tests and this description under my direction. I reviewed the diff and the measurements.
🤖 Generated with Claude Code