tustvold commented on code in PR #6790:
URL: https://github.com/apache/arrow-rs/pull/6790#discussion_r1857250921
##########
arrow-array/src/array/mod.rs:
##########
@@ -365,6 +368,16 @@ impl Array for ArrayRef {
self.as_ref().is_empty()
}
+ fn shrink_to_fit(&mut self) {
+ if let Some(slf) = Arc::get_mut(self) {
+ slf.shrink_to_fit();
+ } else {
+ // TODO(emilk): clone the contents and shrink that.
+ // This can be accomplished if we add `trait Array { fn
clone(&self) -> Box<Array>>; }`.
+ // Or we clone using `let clone = self.slice(0, self.len());` and
hope that the returned `ArrayRef` is not shared.
Review Comment:
Doing nothing seems fine to me
##########
arrow-buffer/src/buffer/immutable.rs:
##########
@@ -167,6 +167,17 @@ impl Buffer {
self.data.capacity()
}
+ /// Shrinks the capacity of the buffer as much as possible, freeing unused
memory.
+ ///
+ /// The capacity of the returned buffer will be the same as [`Self::len`].
+ ///
+ /// If the capacity is already less than or equal to the desired capacity,
this is a no-op.
+ pub fn shrink_to_fit(&mut self) {
+ if self.len() < self.capacity() {
+ *self = Self::from_vec(self.as_slice().to_vec())
+ }
Review Comment:
This will always create a fresh allocation and copy across.
I think this should be a no-op if the buffer is shared or externally
allocated, and otherwise call through to `realloc`. In most cases this will
avoid any data copying at all.
##########
arrow-buffer/src/buffer/offset.rs:
##########
@@ -133,6 +133,12 @@ impl<O: ArrowNativeType> OffsetBuffer<O> {
Self(out.into())
}
+ /// Free up unused memory.
+ #[inline]
Review Comment:
Given these methods are performing memory manipulations, I think it unlikely
`inline` will yield any performance difference and may hurt compile times.
##########
arrow-array/src/array/mod.rs:
##########
@@ -167,6 +167,9 @@ pub trait Array: std::fmt::Debug + Send + Sync {
/// ```
fn is_empty(&self) -> bool;
+ /// Frees up unused memory.
Review Comment:
```suggestion
/// Shrinks the capacity of any exclusively owned buffer as much as
possible
///
/// Shared or externally allocated buffers will be ignored, and
/// any buffer offsets will be preserved.
```
--
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]