alamb commented on code in PR #9025:
URL: https://github.com/apache/arrow-datafusion/pull/9025#discussion_r1468533266
##########
datafusion/execution/src/memory_pool/proxy.rs:
##########
@@ -24,12 +24,18 @@ pub trait VecAllocExt {
/// Item type.
type T;
- /// [Push](Vec::push) new element to vector and store additional allocated
bytes in `accounting` (additive).
+ /// [Push](Vec::push) new element to vector and increase
+ /// `accounting` by any newly allocated bytes.
+ ///
+ /// Note that allocation counts capacity, not size
fn push_accounted(&mut self, x: Self::T, accounting: &mut usize);
- /// Return the amount of memory allocated by this Vec (not
- /// recursively counting any heap allocations contained within the
- /// structure). Does not include the size of `self`
+ /// Return the amount of memory allocated by this Vec to store elements
+ /// (`size_of<T> * capacity`).
+ ///
+ /// Note this calculation is not recursive, and does not include any heap
+ /// allocations contained within the Vec's elements. Does not include the
+ /// size of `self`
Review Comment:
when reviewing this code, I note that datafusion has a custom growth
strategy (2x) which is wasteful with large allocations -- I think we should
perhaps go back to using the built in rust growth strategy. I'll file a PR /
ticket with this idea later
--
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]
