lriggs commented on code in PR #55282:
URL: https://github.com/apache/arrow/pull/55282#discussion_r4459341185
##########
cpp/src/gandiva/llvm_generator.cc:
##########
@@ -1263,6 +1280,12 @@ LValuePtr
LLVMGenerator::Visitor::BuildFunctionCall(const NativeFunction* func,
auto llvm_return_type = types->IRType(arrow_return_type_id);
DecimalIR decimalIR(generator_->engine_.get());
+ // Only functions that get the execution context can allocate from its arena
(directly
+ // or through a function holder), so those need the arena released after
each row.
+ if (func->NeedsContext()) {
+ has_arena_allocs_ = true;
Review Comment:
This has widened which functions fall under has_arena_allocs_. Might be
worth considering/testing if the additional checks add any measureable overhead.
For example 'locate' takes the context for error reporting but never
allocates memory. With this change there will be some conditional branches that
are always false/unused.
Also a lot of math functions hit this as well, through the
BINARY_SYMMETRIC_UNSAFE_NULL_IF_NULL macro registration.
--
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]
