emilk commented on code in PR #6790:
URL: https://github.com/apache/arrow-rs/pull/6790#discussion_r1856877614
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -430,31 +430,31 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
///
/// Before GC:
/// ```text
- /// ┌──────┐
- /// │......│
- /// │......│
- /// ┌────────────────────┐ ┌ ─ ─ ─ ▶ │Data1 │ Large buffer
+ /// ┌──────┐
+ /// │......│
+ /// │......│
+ /// ┌────────────────────┐ ┌ ─ ─ ─ ▶ │Data1 │ Large buffer
/// │ View 1 │─ ─ ─ ─ │......│ with data that
/// ├────────────────────┤ │......│ is not referred
/// │ View 2 │─ ─ ─ ─ ─ ─ ─ ─▶ │Data2 │ to by View 1 or
- /// └────────────────────┘ │......│ View 2
- /// │......│
- /// 2 views, refer to │......│
- /// small portions of a └──────┘
- /// large buffer
+ /// └────────────────────┘ │......│ View 2
+ /// │......│
+ /// 2 views, refer to │......│
+ /// small portions of a └──────┘
+ /// large buffer
/// ```
- ///
+ ///
/// After GC:
///
/// ```text
/// ┌────────────────────┐ ┌─────┐ After gc, only
- /// │ View 1 │─ ─ ─ ─ ─ ─ ─ ─▶ │Data1│ data that is
- /// ├────────────────────┤ ┌ ─ ─ ─ ▶ │Data2│ pointed to by
- /// │ View 2 │─ ─ ─ ─ └─────┘ the views is
- /// └────────────────────┘ left
- ///
- ///
- /// 2 views
+ /// │ View 1 │─ ─ ─ ─ ─ ─ ─ ─▶ │Data1│ data that is
+ /// ├────────────────────┤ ┌ ─ ─ ─ ▶ │Data2│ pointed to by
+ /// │ View 2 │─ ─ ─ ─ └─────┘ the views is
+ /// └────────────────────┘ left
+ ///
+ ///
+ /// 2 views
Review Comment:
Ooops… my editor is set to trim trailing whitspace on save. Let me know if
you want me to reverse.
##########
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:
I think any of the following three approaches makes sense here:
## Do nothing
The current approach. If the `ArrayRef` is shared, then we cannot free up
memory.
## Clone using `slice`
```rs
let mut clone = self.slice(0, self.len());
if let Some(slf) = Arc::get_mut(clone) {
clone.shrink_to_fit();
} else {
// This would be highly unlikely
}
clone
```
## Clone using a new trait method
Add
``` rs
trait Array {
fn clone(&self) -> Box<dyn Array>;
}
(and implement it everywhere)
```
and then call
```rs
let mut clone = Array::clone(self);
clone.shrink_to_fit();
Arc::new(clone)
```
--
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]
