Skip to content

perf: vectorize spark_unscaled_value (9x faster)#4972

Open
u70b3 wants to merge 1 commit into
apache:mainfrom
u70b3:perf/vectorize-unscaled-value
Open

perf: vectorize spark_unscaled_value (9x faster)#4972
u70b3 wants to merge 1 commit into
apache:mainfrom
u70b3:perf/vectorize-unscaled-value

Conversation

@u70b3

@u70b3 u70b3 commented Jul 18, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Part of #4942.

Rationale for this change

spark_unscaled_value is injected by Spark's DecimalAggregates rule for every sum/avg over narrow decimals, so it sits on the TPC-DS decimal aggregation hot path. The array branch used a per-element Int64Builder loop.

What changes are included in this PR?

  • Replace the per-element builder loop with the vectorized Arrow unary kernel: the Decimal128 values buffer is mapped to Int64 in a single pass and the null buffer is carried over untouched (zero-copy). The scalar branch is unchanged.
  • Add a criterion benchmark (benches/unscaled_value.rs) covering no-null / sparse-null (1 in 10) / dense-null (1 in 2) shapes at 8192 rows, following the scalar expression optimization guide.

Benchmark results (8192 rows, Decimal128(20,2), median):

shape before after speedup
no nulls 29.5 µs 3.2 µs ~9.3x
sparse nulls (1/10) 35.5 µs 3.0 µs ~11.7x
dense nulls (1/2) 30.5 µs 2.9 µs ~10.4x

How are these changes tested?

  • Added unit tests for the array path (values and null distribution are bit-exact, including nulls) and the scalar path; the function previously had no direct tests.
  • Full datafusion-comet-spark-expr suite passes (497 tests), plus clippy --all-targets -D warnings and fmt.
  • Benchmark shows a strict improvement on all three shapes with no regression (criterion change −89% to −92%, p=0.00).

@u70b3
u70b3 force-pushed the perf/vectorize-unscaled-value branch from 40a7762 to 81ebfdf Compare July 18, 2026 02:47
Replace the per-element Int64Builder loop in the array branch with the
vectorized Arrow unary kernel: the Decimal128 values buffer is mapped to
Int64 in a single pass and the null buffer is carried over untouched
(zero-copy). The scalar branch is unchanged.

Benchmark (8192 rows, Decimal128(20,2), benches/unscaled_value.rs):
- no nulls:                29.5 us -> 3.2 us (~9.3x)
- sparse nulls (1 in 10):  35.5 us -> 3.0 us (~11.7x)
- dense nulls (1 in 2):    30.5 us -> 2.9 us (~10.4x)

Part of apache#4942
@u70b3
u70b3 force-pushed the perf/vectorize-unscaled-value branch from 81ebfdf to 675d3eb Compare July 18, 2026 04:39

@andygrove andygrove left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks @u70b3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants