Repository navigation
Benchmark the copy of decimal values into the canonical width - #10990
Merged
Merged
Conversation
connortsui20
changed the title
connortsui20
marked this pull request as ready for review
October 29, 2026 19:41
11 of 32 tasks
Merging this PR will improve performance by 11.84%1 warning
Performance Changes
Tip Curious why performance improved? Comment Comparing 
|
Decimal imports and the interleave executor copy values into a wider canonical width through `copy_to_canonical_width`. Measure three widening pairs against a plain same-width copy, so that a change to the widening loop has a baseline. Widening never reads the validity, so the cases have no nulls. How the compiler vectorizes the widening loop depends on the target features, so each case carries `#[cpu_features]` and runs on every walltime CPU-feature leg instead of in simulation. Signed-off-by: Connor Tsui <[email protected]>
connortsui20
force-pushed
the
ct/decimal-widen-copy-bench
branch
from
October 29, 2026 20:33
16050fb to
e7b5ab7
Compare
robert3005
approved these changes
Oct 29, 2026
connortsui20
added a commit
that referenced
this pull request
Oct 29, 2026
## Summary The [CodSpeed report on #10990](#10990 (comment)) flags `test_scan[int_str]` as dominated by system calls. Its trace attributes about 314 ms to condition-variable wakeups under the async I/O reactor. ## Changes Runs Python CPU simulation with `VORTEX_MAX_THREADS=0` and memory-maps the shared benchmark files, so scans execute on the calling thread without dispatching file reads to I/O threads. This changes the scan, filter, and scalar-access benchmark setup, including repeated scans. Pre-commit formatting and sign-off checks passed. Workspace Clippy was started by the push hook and interrupted. Tests and CodSpeed have not been rerun, so clearing the warning still needs CI confirmation. Signed-off-by: Connor Tsui <[email protected]>
connortsui20
added a commit
that referenced
this pull request
Oct 30, 2026
Tracking Issue: #10701 Stacked on #10990, which adds the `copy_to_canonical_width` benchmark first so that CodSpeed measures this change. ## Summary `copy_to_canonical_width` widens decimal values whenever a path needs the canonical array, such as the Arrow and DuckDB imports and interleave. Once DecimalByteParts decodes to a decimal `Widen` (#10946, #10905), every export of a narrow column to DuckDB, or to Arrow above precision 18, pays for this copy. Its loop left some widths scalar, and `i64` to `i128` is now about 1.75x faster on Apple M4. ## Changes `widen_values` collected through `BufferMut::extend_iter`, whose write loop has two exits (the reserved count and the end of the iterator), and its checked cast added a panic edge. LLVM still vectorized `i32` to `i64`, but left other widths scalar, such as `i64` to `i128`. The loop now builds the buffer with `Buffer::from_trusted_len_iter` and falls back to zero on a failed cast, which can't happen when the target is at least as wide as the source, so it has one exit and no panic edge. `widened_buffer` now calls `widen_values` instead of its own copy of the loop. A runtime `assert!` checks the width order, and folds away because both sides are constants. A `const` assert doesn't compile here: the callers dispatch with `if const`, and rustc still evaluates the inline constants of every function that the disabled branches name. Widening into `i256` got 1.24x (`i128` to `i256`) to 1.7x (`i32` to `i256`) slower on Apple M4, and the cause is not known. That only happens at precision 39 to 76, and most such columns will decode to a decimal `Widen` once #10946 and #10905 land, so one loop for every width is the simpler trade. `test_widen_values_sign_extends` checks all 15 widening pairs against a per-value cast, with enough values to run both a vector body and a scalar tail. The CodSpeed simulation run (x86 with AVX2) showed no change in any case, so #10990 now runs the benchmark on the walltime CPU-feature legs, where the NEON leg is the one that matches the M4 result. <details> <summary>Benchmark results (Apple M4)</summary> `cargo bench -p vortex-array --bench copy_to_canonical_width`, 16,384 values with no nulls. Before is the benchmark commit (#10990) and after is the tip of this PR, so the comparison isolates this layer. Each number is the median of three alternating runs of each binary, with 300 samples per run. These numbers come from the earlier version of the benchmark, which also had half-null cases with the same ratios. | case | before (ns) | after (ns) | before / after | | --- | ---: | ---: | ---: | | `i32_to_i64` | 1103 | 1083 | 1.02 | | `i64_to_i128` | 4624 | 2624 | 1.76 | | `i128_to_i256` | 6749 | 8332 | 0.81 | | `same_width_i64` (control) | 1176 | 1177 | 1.00 | </details> ## API Changes None. Signed-off-by: Connor Tsui <[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.
Tracking Issue: #10701
Bottom of stack #10991. #10989 builds on this PR.
Summary
Every path that needs the canonical decimal array widens its values with
copy_to_canonical_width: the Arrow and DuckDB imports, interleave, and (after #10946) the execution of a decimalWiden. No benchmark covered that widening path. This lands before #10989 so that CodSpeed measures the loop change there against it.Changes
Adds a
copy_to_canonical_widthbench that copies 16,384 values fori32toi64,i64toi128andi128toi256, with a same-widthi64copy as the control. Widening never reads the validity, so the cases have no nulls.How the compiler vectorizes the widening loop depends on the target features, so every case carries
#[cpu_features]and runs on the AVX2, AVX-512 and NEON walltime legs instead of in simulation.API Changes
None.