llama90 commented on code in PR #40884:
URL: https://github.com/apache/arrow/pull/40884#discussion_r1573117355
##########
cpp/src/arrow/acero/expression_benchmark.cc:
##########
@@ -66,17 +69,34 @@ static void BindAndEvaluate(benchmark::State& state,
Expression expr) {
ASSIGN_OR_ABORT(auto bound, expr.Bind(*dataset_schema));
ABORT_NOT_OK(ExecuteScalarExpression(bound, input, &ctx).status());
}
+
+ state.counters["rows_per_second"] = benchmark::Counter(
+ static_cast<double>(state.iterations() * num_batches * rows_per_batch),
+ benchmark::Counter::kIsRate);
+
+ state.counters["batches_per_second"] = benchmark::Counter(
+ static_cast<double>(state.iterations() * num_batches),
benchmark::Counter::kIsRate);
}
// A benchmark of SimplifyWithGuarantee using expressions arising from
partitioning.
static void SimplifyFilterWithGuarantee(benchmark::State& state, Expression
filter,
Expression guarantee) {
+ const auto rows_per_batch = static_cast<int32_t>(state.range(0));
+ const auto num_batches = 1000000 / rows_per_batch;
+
auto dataset_schema = schema({field("a", int64()), field("b", int64())});
ASSIGN_OR_ABORT(filter, filter.Bind(*dataset_schema));
for (auto _ : state) {
ABORT_NOT_OK(SimplifyWithGuarantee(filter, guarantee));
}
+
+ state.counters["rows_per_second"] = benchmark::Counter(
+ static_cast<double>(state.iterations() * num_batches * rows_per_batch),
+ benchmark::Counter::kIsRate);
+
+ state.counters["batches_per_second"] = benchmark::Counter(
+ static_cast<double>(state.iterations() * num_batches),
benchmark::Counter::kIsRate);
Review Comment:
I rolled back all the changes related to `expressions_benchmark.cc`. As you
mentioned, the metrics such as `rows_per_second` and `batches_per_second` seem
unnecessary for this benchmark. While working, I was too focused on outputting
these metrics.
Based on your review, it appears that those were incorrect modifications, so
I have rolled back all related changes.
Thank you for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
