Repository navigation
Decode DecimalByteParts into the canonical array or a decimal Widen - #10905
connortsui20 wants to merge 10 commits into
Conversation
Merging this PR will degrade performance by 39.09%1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
59eca0e to
881fe73
Compare
881fe73 to
fca52f8
Compare
|
So the regressions are definitely real, but without narrow decimal encoding there's not much we can do. I will note though that I think it is good we regress these benchmarks because now it actually shows how expensive a very large decimal is in its fully decoded representation. |
fca52f8 to
e0640ce
Compare
|
But we have Widen now so shouldn't this decode to Decimal(Widen) to preserve behaviour? |
e0640ce to
5b77c67
Compare
5b77c67 to
ed4f730
Compare
WD1 to WD4, CD3, CD3b and the decimal Interleave kernel merged, and CD4, CD5 and WD6 are open drafts. The plan now lists CD3b (#10939) and the Interleave kernel (CD4p, #10940) as dependencies of CD4, and CD3b as a dependency of CD6 and CD7. CD7 merges right after CD5 (#10905). D26 moves to CD6, because CD4 (#10946) adds no serialization lowering: the default writer executes a Widen, and reads of old files are the first ordinary source of one. The CD6 and WR1 briefs now carry that choice. D35 no longer makes WD6 wait for WD5. #10908 joins the follow-up issues, and the plan suggests settling the mechanism of Signed-off-by: Connor Tsui <connor.tsui20@gmail.com> #10748 before WD5.
The first version of #10905 widened every narrow DecimalByteParts MSP with a copy, and CodSpeed showed regressions of up to 11x. The maintainer approved a plan on 2026-10-29: decoding returns the canonical array or a decimal Widen, LT2 becomes part of CD5, and CD5 depends on CD4. D9 now has one rule for every stored width, so a wider old node can also become a Widen. D16, D21 and the new D40 make WidenArray::from_stored_decimal the single owner of the choice between the canonical array and a Widen. D26 is settled: CD4 lowers a Widen in the serializer. D31 names 0.91.0, and D36 records the decimal Sum and MinMax kernels. D41 records the order and the stacks, with two new PRs, and D42 the decode benchmarks that stop at a Widen. The implementation plan follows: the PR table, the graph, the waves, the later work, the rules for every PR and the risks. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
ed4f730 to
86ce007
Compare
`widen_values` and `widened_buffer` widened each value with a checked cast and an `expect`, and collected through `BufferMut::extend_iter`, whose write loop has two exits. LLVM vectorized some width pairs, such as `i32` to `i64`, but left others scalar, such as `i64` to `i128`. Build the buffer with `from_trusted_len_iter`, like the Arrow exporter, and fall back to zero instead of panicking. The fallback is dead code when the target is at least as wide as the source, so the loop has one exit and no panic edge. `widened_buffer` now calls `widen_values`. On Apple M4, `i64` to `i128` is about 1.75x faster and `i32` to `i64` is unchanged, but widening into `i256` is 1.24x to 1.7x slower, for a reason we did not find. That widening only happens at precision 39 to 76, and once #10946 and #10905 land, most such columns decode to a `Widen`, so one loop for every width is the simpler trade. A test checks every widening pair against a per-value cast. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
2720140 to
e25d528
Compare
11f665c to
ee2eb96
Compare
ee2eb96 to
5092c84
Compare
5092c84 to
c8c52ef
Compare
c8c52ef to
4c2b19b
Compare
The compressor never narrows a lower part, and no file has lower parts yet, so accepting narrower unsigned lower parts only cost a cast during assembly. Bounding the lower-part count by the canonical width of the dtype also means that decoding never has to narrow assembled values. Every lower part now has the dtype `u64`, and only the MSP narrows. The constructor, `assemble_decimal` and the v2 reader reject other lower part dtypes. Precision up to 18 allows no lower parts, 19 to 38 at most one, and 39 to 76 up to three. `DecimalByteParts::encode` now uses the checked constructor, because storage wider than the canonical width splits into more lower parts than its dtype allows. That was the only caller of the unchecked constructor, so it is removed. The crate doc moves to `//!`, where rustdoc reads it. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Decoding DecimalByteParts with a narrow MSP will return a decimal `Widen` over the MSP instead of widening it into the canonical array. The `decode` and `filter` benches and the `decompress` bench of the encoding trees now stop at a `Widen` with `execute_until`, so they measure what decoding produces and not the widening that only exports need. A new `canonicalize` bench keeps the old `decode`, which executes to the canonical `DecimalArray`, so CodSpeed still tracks the full widening that exports pay. `execute_until` stops at canonical arrays as well, so these benches run on the current decoder too. The other encoding trees never yield a `Widen`, so their `decompress` cases do the same work as before. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Widening every narrow MSP into a new canonical buffer made decoding up to 11x slower in CodSpeed. A decimal `Widen` can reuse the narrow MSP instead, so `assemble_decimal` now hands the assembled values to `WidenArray::from_stored_decimal`, which returns the canonical array or a `Widen` over a narrower child. The parts assemble at their natural width: the MSP width with no lower parts, `i128` with one, and `i256` with two or three. A lone MSP at the canonical width is reused with no scan. Any other lone MSP gets its range from the integer `min_max`, which reads exact cached extrema or makes one vectorized pass, unlike the decimal `valid_range` scan. `from_stored_decimal` scans values assembled from lower parts only when they are narrower than the canonical width, which happens for one lower part at a precision above 38. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
The CPU decoder now returns the canonical array or a `Widen`, so every decimal array that it returns has the canonical width of its own dtype. The CUDA decoder still returned the MSP at its own width. CUDA has no `Widen` kernel, so the GPU decoder widens on the device instead. An MSP at the canonical width is reused with no copy, and a narrower MSP is widened with the decimal_cast kernels. An MSP wider than the canonical width returns an error: narrowing it must check the precision, and that check on the device needs an overflow flag that synchronizes with the host. Older writers can store such an MSP, but the CUDA Arrow export already rejected those columns. Lower parts still return an error on the GPU. The device widening moves out of the Arrow export into a crate-private kernel module, so the export and the decoder share one entry point that rejects narrowing. The decimal_cast kernels gain an i16 output, because an i8 MSP at precision 3 or 4 widens to i16. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
4c2b19b to
b6a0182
Compare
Tracking Issue: #10701
Stacked on #11005. #10992 builds on it. It does not need #11006.
Summary
A narrow DecimalByteParts MSP should remain narrow after decoding, while every decimal leaf must have the canonical width of its own dtype. Returning a
Widenpreserves both rules and avoids a widening copy on the common read path.Merge gates:
uv run vortex-test/compat-gen/scripts/compat.py check --mode laston this branch.Changes
Decoding shares
from_stored_decimalwith the other decimal reader. A lone MSP supplies exact cached extrema or one integer MinMax pass, and the selected child caches those extrema for later aggregates. Lower parts must beu64, with their count bounded by precision, so only a lone MSP can exceed the canonical width.CUDA has no
Widenkernel, so it widens a narrow MSP on the device. Device narrowing remains an error because overflow detection would require a host synchronization. Decode benchmarks stop at the decoded form, with a separate canonicalization benchmark for the widening cost.Historical local benchmark results (Apple M4, medians)
These retained measurements predate this style pass and rebase. They do not measure the current head.
cargo bench -p vortex-decimal-byte-parts --bench dbp_compute, 8,192 rows. Before is the tip of #10946 before it was split into #11002 to #11006, which decoded DecimalByteParts asdevelopdid at the time. After is the earlier implementation that produced these measurements. The results isolate this PR.msp_i64_p18_canonicalis the control: its MSP is already at the canonical width, so this PR does not change its decode.decodegap_msp_i16_p10decodemsp_i32_p18decodemsp_i32_p18_nullabledecodemsp_i64_p18_canonicalcanonicalize(new)gap_msp_i16_p10canonicalize(new)msp_i32_p18canonicalize(new)msp_i32_p18_nullablefiltergap_msp_i16_p10filtermsp_i32_p18filtermsp_i32_p18_nullablefiltermsp_i64_p18_canonicalmin_maxgap_msp_i16_p10min_maxmsp_i32_p18min_maxmsp_i32_p18_nullableBenches that this PR does not change, such as
compare_constantand the control, ran about 20% slower in the after run than in the before run, so part of each after number is run-to-run noise.canonicalizeincludes the widening cost that the narrow MSP decode avoids.API Changes
DecimalByteParts decoding returns a canonical decimal array or a decimal
Widen. Constructors and the v2 reader requireu64lower parts and reject counts beyond the precision's width. CUDA decoding returns the canonical width and rejects a wider MSP.