Skip to content

fix(cuda): copy encoded device-resident children to the host in into_host - #10916

Merged
0ax1 merged 2 commits into
vortex-data:developfrom
TanishGudise:tg/pr2-into-host
Nov 10, 2026
Merged

0ax1 merged 2 commits into
vortex-data:developfrom
TanishGudise:tg/pr2-into-host

Conversation

@TanishGudise

Copy link
Copy Markdown
Contributor

Summary

compress-bench --gpu-decompress --gpu-verify panics with expected host buffer on develop for CMSprovider and Euro2016. A run-end encoded validity mask stays encoded on the device after execute_cuda. CanonicalCudaExt::into_host then 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 take through device-resident codes, fails the same way.

Changes

  • canonical.rs: child_into_host copies every device buffer of a child that isn't canonical to the host before executing it 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 (Sparse rejects buffer replacement).
    • The decode path is unchanged.
    • This doesn't add mixed CPU/GPU execution (gpu-scan-cli scan --gpu-file panics with "as_slice must be called on host buffer" #7838): the child moves to the host whole, so nothing decodes from device buffers on the host.
  • Tests (4), each building the child on the device and comparing into_host with the expected result:
    • a primitive array whose validity is a run-end encoded mask;
    • one whose validity is a sparse mask (Sparse keeps its fill value in a host buffer it doesn't allow to be replaced, so only its children move);
    • a string dictionary with nullable values and device-resident codes;
    • a primitive dictionary with nullable values and device-resident codes.

Evidence (A40, CUDA 13.0):

Check develop this PR
The 4 tests all fail: expected host buffer (run-end mask), as_slice must be called on host buffer (sparse mask, both dictionaries) 4 pass
--gpu-verify on CMSprovider and Euro2016, today's preset both panic both pass (234 and 22 fields verified)
vortex-cuda lib suite — 711 passed, 0 failed

Everything in the table ran on this PR alone. With all three PRs, --gpu-verify passes 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

…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]>
@codspeed

codspeed Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

✅ 1964 untouched benchmarks


Comparing TanishGudise:tg/pr2-into-host (53d47bb) with develop (bed9e0c)

Open in CodSpeed

@joseph-isaacs
joseph-isaacs requested a review from 0ax1 October 29, 2026 15:42
@0ax1 0ax1 added the changelog/fix A bug fix label Nov 6, 2026

@0ax1 0ax1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good! Thanks again for your contribution!

@0ax1
0ax1 merged commit ced8ec9 into vortex-data:develop Nov 10, 2026
72 checks passed
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants