Skip to content

perf: optimize spark_floor (up to 4x faster)#4911

Open
andygrove wants to merge 2 commits into
apache:mainfrom
andygrove:auto-opt/spark_floor-datafusion-comet-20260713-114953
Open

perf: optimize spark_floor (up to 4x faster)#4911
andygrove wants to merge 2 commits into
apache:mainfrom
andygrove:auto-opt/spark_floor-datafusion-comet-20260713-114953

Conversation

@andygrove

@andygrove andygrove commented Jul 13, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

Optimize an existing expression.

What changes are included in this PR?

Decimal floor now monomorphizes the per-element op (was &dyn Fn) and dispatches on input scale to a const divisor with an i64 fast path, so the per-row 128-bit __divti3 division libcall becomes a multiply-and-shift.

How are these changes tested?

Existing tests.

Benchmark (criterion):

  • spark_floor_ float64: 10.548% faster (base 1525ns -> cand 1364ns)
  • spark_floor_ decimal128(38,6) wide values: 3.043% faster (base 38997ns -> cand 37810ns)
  • spark_floor_ decimal128(38,2): 69.852% faster (base 25462ns -> cand 7676ns)
  • spark_floor_ decimal128(38,18): 75.222% faster (base 28693ns -> cand 7109ns)

Full criterion output:

spark_floor: decimal128(38,2)
                        time:   [7.6379 µs 7.6620 µs 7.6869 µs]
                        change: [−69.961% −69.852% −69.739%] (p = 0.00 < 0.05)
                        Performance has improved.

spark_floor: decimal128(38,18)
                        time:   [7.0976 µs 7.1216 µs 7.1451 µs]
                        change: [−75.295% −75.222% −75.141%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) low mild

spark_floor: decimal128(38,6) wide values
                        time:   [37.480 µs 37.607 µs 37.739 µs]
                        change: [−3.4647% −3.0427% −2.6266%] (p = 0.00 < 0.05)
                        Performance has improved.

spark_floor: float64    time:   [1.3466 µs 1.3669 µs 1.3899 µs]
                        change: [−12.450% −10.548% −8.3898%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  7 (7.00%) high mild
  5 (5.00%) high severe

@andygrove andygrove changed the title perf: optimize spark_floor in datafusion-comet-spark-expr perf: optimize spark_floor (up to 4x faster) Jul 13, 2026
@andygrove andygrove marked this pull request as ready for review July 13, 2026 18:18
…datafusion-comet-20260713-114953

# Conflicts:
#	native/spark-expr/Cargo.toml
@mbutrovich

Copy link
Copy Markdown
Contributor

Both this and #4926 edit the shared make_decimal_array signature in utils.rs. #4911 changes it to make_decimal_array<F: Fn(i128)->i128>(... f: F); #4926 changes the same line to f: impl Fn(i128)->i128. These two edits conflict on merge, and worse the two PRs solve the same problem two different ways: #4911 dispatches on a compile-time EXP const (dispatch_pow10! + decimal_floor_pow10), while #4926 does a per-row runtime i64::try_from branch inside decimal_ceil_f (ceil.rs:79). Floor and ceil are the same operation with the sign flipped and should share one mechanism.

Suggested change: land one mechanism and apply it to both. The dispatch_pow10! macro already lives in utils.rs and is written generically, so spark_ceil should adopt it with a decimal_ceil_pow10::<EXP> mirror of decimal_floor_pow10. Coordinate the make_decimal_array signature so only one of the two forms lands (impl Fn and <F: Fn> are equivalent here, pick one). Sequence the two PRs so the second rebases onto the first.

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