feat(btrblocks): compress binary arrays with VarBin offsets and FSST - #9574
Closed
joseph-isaacs wants to merge 3 commits into
Closed
feat(btrblocks): compress binary arrays with VarBin offsets and FSST#9574joseph-isaacs wants to merge 3 commits into
joseph-isaacs wants to merge 3 commits into
Conversation
Canonical binary arrays are VarBinViewArray, which spends a fixed 16 bytes per element on an opaque views buffer. No scheme could compress that buffer, so any binary column that the dictionary scheme declined was written as payload plus 16 B/value, regardless of content. VarBinScheme re-encodes as VarBinArray, replacing the views buffer with an offsets child array that the cascading compressor compresses with the ordinary integer schemes. For fixed-width values the offsets are a constant-stride sequence and collapse to nothing. This mirrors what FSSTScheme already does for strings. Measured at 100k rows (tests/varbin_scheme.rs), compressed nbytes against the same compressor with the scheme excluded: nulls every 7th 2,966,827 -> 1,647,348 (0.56) random 16B (hash) 3,200,000 -> 1,600,000 (0.50) shared prefix 3,200,000 -> 1,600,000 (0.50) random 256B 27,200,000 -> 25,600,000 (0.94) The one golden snapshot that moves also improves: binary_low_cardinality dictionary values go from 96 to 52 bytes as the scheme cascades into the dictionary's values child. Checks: cargo test -p vortex-btrblocks (all pass), cargo test -p vortex-file (144 pass), cargo clippy -p vortex-btrblocks --all-targets --all-features (clean), cargo +nightly fmt --all. Signed-off-by: "Claude" <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GJdgPga43u5rXYwv6R5b19
FSSTScheme gated on `is_utf8()`, but its compress path never validates UTF-8 -- it trains and compresses over the raw `array_as_varbinview()` bytes. The gate therefore excluded binary columns from a scheme that already works on them, leaving any binary payload with intra-value structure (shared prefixes, zero padding, common framing) uncompressed once the dictionary scheme declined it. Widening `matches` to accept binary, measured at 100k rows (tests/varbin_scheme.rs), compressed nbytes: shared prefix 1,600,000 -> 734,685 nulls every 7th 1,647,348 -> 690,230 random 16B (hash) 1,600,000 -> 1,600,000 (VarBinScheme still selected) random 256B 25,600,000 -> 25,600,000 (VarBinScheme still selected) Binary now lands byte-identical to the same content stored as Utf8 (734,685 either way), which is what confirms the dtype gate was not protecting anything. The two schemes compose rather than compete: on incompressible payloads FSST alone is worse than VarBinScheme (1,872,028 vs 1,600,000 for random 16B) because the symbol table buys nothing, and scheme selection picks VarBinScheme there. Not measured: symbol-table training cost on write and FSST decode cost on read. The `is_utf8()` restriction may also have had a rationale outside the compress path that this change does not account for, so the gate's history is worth checking before relying on this. Checks: cargo test -p vortex-btrblocks (all pass, including the roundtrip assertions which now exercise FSST on binary), cargo test -p vortex-file (144 pass), cargo clippy -p vortex-btrblocks --all-targets --all-features (clean), cargo +nightly fmt --all. golden_default is unchanged. Signed-off-by: "Claude" <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GJdgPga43u5rXYwv6R5b19
The two binary-scheme tests printed their measurements without asserting anything, so a regression would have shown up only on a careful read of the output. Add the two invariants the schemes are meant to hold: - enabling VarBinScheme never grows the output, so a future selection change that makes it lose is a test failure rather than a silent regression; - FSST compresses bytes rather than codepoints, so the same values must compress identically whether typed as binary or utf8. Signed-off-by: "Claude" <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GJdgPga43u5rXYwv6R5b19
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | words_gather_dispatch_avx2[1024] |
17 ns | 19 ns | -10.53% |
| ⚡ | Simulation | compact[(2048, 90)] |
1.8 µs | 1.6 µs | +12.07% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/paimon-retreat-signal-3wp176 (084d21d) with develop (62648ef)
Footnotes
-
54 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
|
at some point we definitely had fsst and other things for binaries, when did that get lost? |
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.
Superseded by #9576 — same commits, reopened from a correctly-named branch.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GJdgPga43u5rXYwv6R5b19