[GitHub] [arrow] zhztheplayer commented on a diff in pull request #34231: GH-34230: [Java] Call allocation listener on BaseAllocator#wrapForeignAllocation

["zhztheplayer (via GitHub)" <gi...@apache.org>]

zhztheplayer commented on code in PR #34231:
URL: https://github.com/apache/arrow/pull/34231#discussion_r1111383894


##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java:
##########
@@ -233,6 +233,41 @@ private void childClosed(final BaseAllocator childAllocator) {
     listener.onChildRemoved(this, childAllocator);
   }
 
+  @Override
+  public ArrowBuf wrapForeignAllocation(ForeignAllocation allocation) {
+    assertOpen();
+    final long size = allocation.getSize();
+    listener.onPreAllocation(size);
+    AllocationOutcome outcome = this.allocateBytes(size);

Review Comment:
   Thanks for noticing this. I think it's not a very big issue as long as we treat "accountation of foreign allocation" as another type of "allocation" too. Since sometimes it can be tricky to track on actual foreign allocation at real-time from Java side.
   
   Also another solution would be just excluding foreign allocation from `listener.release()` too. But in that way one will lose tracking if he didn't set up a independent path to report the foreign allocations.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org