[connortsui20]

Summary

• Tracking Issue: Tracking Issue: Fix Canonical Decimal #10701

Every decimal leaf must have the canonical width of its precision, but DecimalByteParts usually stores its most significant part (MSP) narrower than that. Copying the MSP to the canonical width makes every read pay for the copy: an earlier version of this PR did that, and CodSpeed showed decode[msp_i32_p18_nullable] going from 18.7 µs to 218.8 µs. Decoding to a decimal Widen keeps reusing the narrow MSP buffer, as develop does today.

Changes

The child precision comes from the values through WidenArray::from_stored_decimal, which the vortex.decimal reader (#10995) also uses, because a width does not determine a precision (32,767 fits in an i16 but needs precision 5), and a child labeled too low breaks compare shortcuts and casts. Known cost: without exact cached MSP statistics, decoding makes one pass to find the range. It uses the integer min_max on the MSP, since the decimal range scan is not vectorized, and the child caches the result for later aggregates.

Bounding the lower parts means assembly never needs a narrowing copy, and no file breaks, since no edition permits lower parts yet.

CUDA has no Widen kernel, so the GPU still widens a narrow MSP and rejects a wider one, because the overflow check would need a host sync. This needs a CUDA reviewer.

Merge #10992 right after this.

Local benchmark results from an earlier revision

The decode benchmarks stop at the Widen to measure the read path, and the new canonicalize benchmark measures the widening copy that consumers of the canonical array pay.

cargo bench -p vortex-decimal-byte-parts --bench dbp_compute on an Apple M4, 8,192 rows, medians. The baseline is the tip of #10946 before its split, which decoded DecimalByteParts as develop does. The candidate is a revision of this PR from before the cleanup and rebase, so these numbers do not measure the current head. msp_i64_p18_canonical is the control, because its MSP already has the canonical width. Benchmarks that this PR does not change, such as compare_constant, ran about 20% slower in the candidate run, so part of each difference is run-to-run noise.

bench	case	baseline	candidate
decode	gap_msp_i16_p10	82.75 ns	1.29 µs
decode	msp_i32_p18	73.97 ns	1.03 µs
decode	msp_i32_p18_nullable	80.49 ns	5.332 µs
decode	msp_i64_p18_canonical	72.02 ns	89.28 ns
canonicalize (new)	gap_msp_i16_p10		2.519 µs
canonicalize (new)	msp_i32_p18		2.041 µs
canonicalize (new)	msp_i32_p18_nullable		6.957 µs
filter	gap_msp_i16_p10	749.7 ns	1.791 µs
filter	msp_i32_p18	1.041 µs	2.04 µs
filter	msp_i32_p18_nullable	2.582 µs	6.291 µs
filter	msp_i64_p18_canonical	1.999 µs	2.249 µs
min_max	gap_msp_i16_p10	5.124 µs	1.958 µs
min_max	msp_i32_p18	4.083 µs	1.739 µs
min_max	msp_i32_p18_nullable	6.165 µs	5.957 µs

API Changes

DecimalByteParts decoding returns a canonical decimal array or a decimal Widen instead of a narrow leaf. The constructors and the v2 reader reject lower parts that are not u64 or that assemble wider than the canonical width.

[codspeed]

Merging this PR will degrade performance by 57.42%

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

❌ 14 regressed benchmarks
✅ 1756 untouched benchmarks
🆕 4 new benchmarks
⏩ 47 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	CPU Simulation	decode[msp_i32_p18_nullable]	28.2 µs	235.5 µs	-88.04%
❌	CPU Simulation	decode[msp_i64_p38]	23.1 µs	141.1 µs	-83.65%
❌	CPU Simulation	decode[gap_msp_i16_p10]	23 µs	129.6 µs	-82.25%
❌	CPU Simulation	decode[msp_i32_p18]	23.2 µs	109.1 µs	-78.76%
❌	CPU Simulation	sum[gap_msp_i16_p10]	83.3 µs	173.6 µs	-52%
❌	CPU Simulation	sum[msp_i32_p18_nullable]	162.9 µs	331.3 µs	-50.84%
❌	CPU Simulation	filter[msp_i32_p18_nullable]	154.3 µs	277.3 µs	-44.36%
❌	CPU Simulation	filter[gap_msp_i16_p10]	98 µs	170.1 µs	-42.37%
❌	CPU Simulation	sum[msp_i32_p18]	97.3 µs	152.2 µs	-36.08%
❌	CPU Simulation	filter[msp_i32_p18]	119.6 µs	175 µs	-31.67%
❌	CPU Simulation	sum[msp_i64_p38]	145 µs	208.2 µs	-30.38%
❌	CPU Simulation	min_max[gap_msp_i16_p10]	108.5 µs	151.6 µs	-28.44%
❌	CPU Simulation	filter[msp_i64_p38]	162.3 µs	224.1 µs	-27.57%
❌	CPU Simulation	decompress[dbp_bp_i32]	61 µs	83.3 µs	-26.85%
🆕	CPU Simulation	canonicalize[gap_msp_i16_p10]	N/A	205.5 µs	N/A
🆕	CPU Simulation	canonicalize[msp_i32_p18_nullable]	N/A	340.2 µs	N/A
🆕	CPU Simulation	canonicalize[msp_i32_p18]	N/A	186.1 µs	N/A
🆕	CPU Simulation	canonicalize[msp_i64_p38]	N/A	332 µs	N/A

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing ct/dbp-canonical-decode (490ca09) with develop (068d819)}

Footnotes

1. 47 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. ↩

[connortsui20]

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.

[robert3005]

But we have Widen now so shouldn't this decode to Decimal(Widen) to preserve behaviour?

[connortsui20]

I have been redesigning this, so this is no longer the change I want to make.