projjal commented on code in PR #13446:
URL: https://github.com/apache/arrow/pull/13446#discussion_r965833974
##########
java/gandiva/src/main/java/org/apache/arrow/gandiva/evaluator/JavaSecondaryCacheInterface.java:
##########
@@ -36,9 +36,11 @@ public BufferResult(long address, long size) {
}
- BufferResult getSecondaryCache(long addr, long size);
+ BufferResult get(long addr, long size);
- void setSecondaryCache(long addrExpr, long sizeExpr, long addr, long size);
+ void set(long addrExpr, long sizeExpr, long addr, long size);
Review Comment:
Can you add comments before the methods that also describes the parameters?
Also rename the parameters to more clear names..like keyBufAddr, keyBufSize,
valueBufAddr, valueBufSize
##########
cpp/src/gandiva/jni/jni_common.cc:
##########
@@ -663,7 +665,17 @@ std::shared_ptr<arrow::Buffer> JavaSecondaryCache::Get(
auto data = std::shared_ptr<arrow::Buffer>(
new arrow::Buffer(reinterpret_cast<uint8_t*>(ret_address), ret_size));
- return data;
+ // copy the buffer and release the original memory
+ auto result =
+ arrow::Buffer::CopyNonOwned(*data.get(),
arrow::default_cpu_memory_manager());
Review Comment:
how is this memory released back?
##########
cpp/src/gandiva/jni/jni_common.cc:
##########
@@ -663,7 +665,17 @@ std::shared_ptr<arrow::Buffer> JavaSecondaryCache::Get(
auto data = std::shared_ptr<arrow::Buffer>(
Review Comment:
you can just stack allocate 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]
