feat(memory-tracking): expose API to NullBuffer, ArrayData, and Array#8918
feat(memory-tracking): expose API to NullBuffer, ArrayData, and Array#8918notfilippo wants to merge 2 commits intoapache:mainfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
cc @alamb since you reviewed the original PR. cc @waynexia / @Dandandan as the original authors. |
|
Thanks @notfilippo -- I don't have time to drive this type of change now (low level memory allocation tracking) -- I am overwhelmed by trying to keep things going. Hopefully some of the other committers will have time to help |
| /// ``` | ||
| #[cfg(feature = "pool")] | ||
| fn claim(&self, pool: &dyn arrow_buffer::MemoryPool) { | ||
| self.to_data().claim(pool) |
There was a problem hiding this comment.
I think calling to_data allocates a new Vec to create the ArrayData (the child buffers specifically)
Should we perhaps instead thread the claim API through the actual arrays / buffers?
There was a problem hiding this comment.
Sounds good! Updating my PR
There was a problem hiding this comment.
Here maybe I should add a comment stating that Array impl can provide a more optimised method. Wdyt?
|
Thanks @notfilippo -- I am sorry for the delay in reviewing -- I am bummed no one else had a chance yet. This API also came up today with @cetra3 and @adriangb in the context of DataFusion If we can avoid the call to |
|
I have another PR open which does similar things, and avoids the Edit: actually I lie about the |
New `claim` API for NullBuffer, ArrayData, and Array. New `pool` feature-flag to arrow, arrow-array and arrow-data. Part of apache#8137. Replaces apache#8040.
notfilippo
force-pushed
the
filippo.rossi/memory-tracking-expose-api
branch
from
ab67828 to
e73b4cb
Compare
|
I rebased the PR and I'll try to look into implementing |
|
Done! |
notfilippo
force-pushed
the
filippo.rossi/memory-tracking-expose-api
branch
from
43f2888 to
176e7c1
Compare
3 participants
Which issue does this PR close?
Part of #8137. Follow up of #7303. Replaces #8040.
Rationale for this change
#7303 implements the fundamental symbols for tracking memory. This patch exposes those APIs to a higher level Array and ArrayData.
What changes are included in this PR?
New
claimAPI for NullBuffer, ArrayData, and Array. Newpoolfeature-flag to arrow, arrow-array, and arrow-data.Are these changes tested?
Added a doctest on the
Array::claimmethod.Are there any user-facing changes?
Added API and a new feature-flag for arrow, arrow-array, and arrow-data.