andygrove commented on code in PR #6048:
URL: https://github.com/apache/datafusion-comet/pull/6048#discussion_r4054261775
##########
spark/src/main/scala/org/apache/comet/Tracing.scala:
##########
@@ -23,6 +23,16 @@ object Tracing {
private val nativeLib = new Native
+ /**
+ * Emits the Arrow memory counters for the JVM side. `jvm_arrow_imported` is
the part of
+ * `jvm_arrow_allocated` that came from native over the C Data Interface, so
the difference is
+ * the Arrow memory the JVM allocated itself. See
[[CometArrowImportAllocator]].
+ */
+ def logArrowMemory(): Unit = {
+ nativeLib.logMemoryUsage("jvm_arrow_allocated",
CometArrowAllocator.getAllocatedMemory)
+ nativeLib.logMemoryUsage("jvm_arrow_imported",
CometArrowImportAllocator.getAllocatedMemory)
Review Comment:
Taking your first option here, in 10fff7090. Full response in
https://github.com/apache/datafusion-comet/pull/6048#issuecomment-5744441037.
Origin tracking is not reachable without vendoring: `wrapForeignAllocation`
routes through the same
`allocateBytes` / `onAllocation` / `releaseBytes` path as ordinary
allocation, so an
`AllocationListener` cannot tell the two apart, and
`ForeignAllocation.release0()` and
`memoryAddress()` are both protected, so a wrapper that could hook the
release has to live in
`org.apache.arrow.memory`. So both counters are now documented as allocator
charges, with no
allocation-origin bound and no guaranteed overlap with `native_allocated`,
in the tracing guide, the
memory management guide and both scaladocs. The PR description no longer
carries the subtraction
claim either.
The identity case is also fixed rather than only documented, since the
transfer is mine and should
not make attribution worse for a shape it was not aimed at: the transfer now
skips a result that is
reference-identical to an input. A result that merely shares buffers with an
input still transfers,
and that is named in the docs as an illustration of the contract. The
return-input regression is in
`CometUdfBridgeSuite`; it fails at 128 of 16384 bytes with the skip mutated
out, matching your
measurement.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
