mzabaluev commented on code in PR #8686:
URL: https://github.com/apache/arrow-rs/pull/8686#discussion_r2454345319
##########
arrow-row/src/lib.rs:
##########
@@ -908,14 +908,18 @@ impl RowConverter {
/// [RowConverter]. It will panic if any rows are null. Operations on the
returned [Rows] may
/// panic if the data is malformed.
pub fn from_binary(&self, array: BinaryArray) -> Rows {
+ let (offsets, values, nulls) = array.into_parts();
assert_eq!(
- array.null_count(),
+ nulls.map(|n| n.null_count()).unwrap_or(0),
0,
"can't construct Rows instance from array with nulls"
);
+ let offsets = offsets.iter().map(|&i| i.as_usize()).collect();
+ // Try zero-copy, if it does not succeed, fall back to copying the
values.
+ let buffer = values.into_vec().unwrap_or_else(|values|
values.to_vec());
Rows {
- buffer: array.values().to_vec(),
- offsets: array.offsets().iter().map(|&i| i.as_usize()).collect(),
+ buffer,
+ offsets,
Review Comment:
> resulting in less heap fragmentation.
This is hypothetical, and there is a new vector with typecast offsets
allocated in that same expression. But in general, I think it's better to drop
allocations that end in this scope before making new allocations that survive
it.
--
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]
