dmitry-chirkov-dremio commented on code in PR #49900:
URL: https://github.com/apache/arrow/pull/49900#discussion_r3583853363
##########
cpp/src/gandiva/tests/micro_benchmarks.cc:
##########
@@ -450,6 +450,29 @@ static void TimedTestExprCompilation(benchmark::State&
state) {
}
}
+static void TimedTestNonBitcodeExprCompilation(benchmark::State& state, bool
use_cache) {
+ int32_t iteration = 0;
+ for (auto _ : state) {
+ // schema for input fields
+ double literal_value = use_cache ? 1.0 : static_cast<double>(iteration);
+ auto seed = TreeExprBuilder::MakeLiteral(literal_value);
+ auto schema = arrow::schema({});
+
+ // output field
+ auto field_sin = field("c1", float64());
+
+ // seed is different for each iteration so that cache won't be hit
+ auto sin_func = TreeExprBuilder::MakeFunction("sin", {seed}, float64());
Review Comment:
This benchmark doesn't seem to isolate the path changed by the PR.
`sin` is implemented in precompiled IR (`precompiled/extended_math_ops.cc`),
so this still exercises the bitcode load/link path rather than the external
C-function registration path that `ExternalCFunctions::AddMappings()` now
filters. As written, the benchmark can miss whether selective C-function
mapping helps at all.
Could we switch this to a function that is actually backed by
`GetCFunctions()` so the measurement matches the optimization being introduced
here?
--
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]
