Skip to content

Commit 5c7d33a

Browse files
authored
bench: Fix panic in power benchmark (#26028)
## Which issue does this PR close? - N/A ## Rationale for this change #22651 removed the `power(decimal, int)` code path, but left the benchmarks still passed `Decimal128` x `Int64` arguments, resulting in a panic if the benchmarks were actually run. Rewrite the benchmark to use Float64 inputs instead, with 10% nulls. ## What changes are included in this PR? See above. ## What is the testing strategy for this PR? Benchmark code only. ## Are there any user-facing changes? No.
1 parent 287b4da commit 5c7d33a

1 file changed

Lines changed: 50 additions & 59 deletions

File tree

  • datafusion/functions/benches/math_expressions

‎datafusion/functions/benches/math_expressions/power.rs‎

Lines changed: 50 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -15,34 +15,38 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
//! Microbenchmark for `power(decimal_array, int_*)`.
18+
//! Microbenchmark for `power`.
1919
//!
20-
//! Covers both array- and scalar-shaped integer exponents on a Decimal
21-
//! base. Both shapes are dispatched to the native per-row decimal kernel;
22-
//! the bench guards against any future change that routes either shape
23-
//! through a Float64 round-trip, which is measurably slower than the
24-
//! decimal kernel for the cases the kernel can handle.
20+
//! `power` coerces both arguments to Float64, so the inputs are Float64.
21+
//! Covers a different exponent for each row, as in `power(x, y)`, and a
22+
//! constant exponent, as in `power(x, 2)`.
2523
26-
use arrow::array::{Decimal128Array, Int64Array};
24+
use std::hint::black_box;
25+
use std::ops::Range;
26+
use std::sync::Arc;
27+
28+
use arrow::array::Float64Array;
2729
use arrow::datatypes::{DataType, Field, FieldRef};
2830
use criterion::{Criterion, criterion_group};
2931
use datafusion_common::ScalarValue;
3032
use datafusion_common::config::ConfigOptions;
3133
use datafusion_expr::{ColumnarValue, ScalarFunctionArgs, ScalarUDF};
3234
use datafusion_functions::math::power;
33-
use std::hint::black_box;
34-
use std::sync::Arc;
35+
use rand::rngs::StdRng;
36+
use rand::{Rng, SeedableRng};
3537

36-
fn make_decimal_array(size: usize, precision: u8, scale: i8) -> Decimal128Array {
37-
// Use a fixed unscaled value (250) so the bench is independent of `scale`.
38-
// The four-arm dispatch in `power` only cares about the Decimal variant
39-
// and the exponent's shape, not the numeric value.
40-
let arr = Decimal128Array::from(vec![250i128; size]);
41-
arr.with_precision_and_scale(precision, scale).unwrap()
42-
}
38+
const NULL_DENSITY: f64 = 0.1;
4339

44-
fn make_int_array(size: usize, value: i64) -> Int64Array {
45-
Int64Array::from(vec![value; size])
40+
fn make_f64_array(rng: &mut StdRng, size: usize, range: Range<f64>) -> Float64Array {
41+
(0..size)
42+
.map(|_| {
43+
if rng.random_bool(NULL_DENSITY) {
44+
None
45+
} else {
46+
Some(rng.random_range(range.clone()))
47+
}
48+
})
49+
.collect()
4650
}
4751

4852
fn run_power(
@@ -69,54 +73,41 @@ fn run_power(
6973
fn criterion_benchmark(c: &mut Criterion) {
7074
let power_fn = power();
7175
let config_options = Arc::new(ConfigOptions::default());
72-
let precision: u8 = 20;
73-
let scale: i8 = 2;
74-
let decimal_ty = DataType::Decimal128(precision, scale);
75-
76-
// Exponents are bounded by what the native decimal kernel can handle
77-
// without overflowing the i128 intermediate; see
78-
// <https://github.com/apache/datafusion/issues/22480>
79-
let exponents = [2i64, 4, 8];
76+
let base_field: FieldRef = Field::new("base", DataType::Float64, true).into();
77+
let exp_field: FieldRef = Field::new("exp", DataType::Float64, true).into();
78+
let return_field: FieldRef = Field::new("r", DataType::Float64, true).into();
79+
let arg_fields = vec![base_field, exp_field];
8080

8181
for size in [1024usize, 8192] {
82-
let base_arr = Arc::new(make_decimal_array(size, precision, scale));
83-
let base_field: FieldRef = Field::new("base", decimal_ty.clone(), true).into();
84-
let exp_field: FieldRef = Field::new("exp", DataType::Int64, true).into();
85-
let return_field: FieldRef = Field::new("r", decimal_ty.clone(), true).into();
86-
let arg_fields = vec![base_field, exp_field];
82+
let mut rng = StdRng::seed_from_u64(42);
83+
// Bases are positive: a zero base with a negative exponent is an error.
84+
let base_arr = Arc::new(make_f64_array(&mut rng, size, 0.1..100.0));
85+
let exp_arr = Arc::new(make_f64_array(&mut rng, size, -4.0..4.0));
8786

88-
for &exp in &exponents {
89-
let exp_arr = Arc::new(make_int_array(size, exp));
90-
let array_args = vec![
91-
ColumnarValue::Array(base_arr.clone()),
92-
ColumnarValue::Array(exp_arr),
93-
];
94-
c.bench_function(
95-
&format!(
96-
"power decimal({precision},{scale}) array x int array, exp={exp}, n={size}"
97-
),
98-
|b| {
99-
b.iter(|| {
100-
run_power(
101-
&power_fn,
102-
&array_args,
103-
&arg_fields,
104-
&return_field,
105-
&config_options,
106-
size,
107-
)
108-
})
109-
},
110-
);
87+
let array_args = vec![
88+
ColumnarValue::Array(Arc::clone(&base_arr) as _),
89+
ColumnarValue::Array(exp_arr),
90+
];
91+
c.bench_function(&format!("power f64 array x f64 array, n={size}"), |b| {
92+
b.iter(|| {
93+
run_power(
94+
&power_fn,
95+
&array_args,
96+
&arg_fields,
97+
&return_field,
98+
&config_options,
99+
size,
100+
)
101+
})
102+
});
111103

104+
for exp in [2.0, 0.5] {
112105
let scalar_args = vec![
113-
ColumnarValue::Array(base_arr.clone()),
114-
ColumnarValue::Scalar(ScalarValue::Int64(Some(exp))),
106+
ColumnarValue::Array(Arc::clone(&base_arr) as _),
107+
ColumnarValue::Scalar(ScalarValue::Float64(Some(exp))),
115108
];
116109
c.bench_function(
117-
&format!(
118-
"power decimal({precision},{scale}) array x int scalar, exp={exp}, n={size}"
119-
),
110+
&format!("power f64 array x f64 scalar, exp={exp}, n={size}"),
120111
|b| {
121112
b.iter(|| {
122113
run_power(

0 commit comments

Comments
 (0)