adriangb commented on code in PR #11235:
URL: https://github.com/apache/arrow-rs/pull/11235#discussion_r4221766438
##########
parquet/src/util/push_buffers.rs:
##########
@@ -180,15 +185,156 @@ impl PushBuffers {
new_buffers.push(buffer.clone());
}
}
+ self.buffered_bytes = new_ranges.iter().map(|r| r.end - r.start).sum();
self.ranges = new_ranges;
self.buffers = new_buffers;
}
+ /// Remove all buffered bytes in `ranges`, whatever the shape of the
+ /// pushed buffers.
+ ///
+ /// A buffer that overlaps a range is trimmed or split, and the parts
+ /// outside the range are kept. The kept parts are zero-copy slices. Thus,
+ /// the allocator frees the memory of a pushed [`Bytes`] only after all of
+ /// its parts are removed.
+ ///
+ /// If the buffers are sorted by start, they stay sorted. The order of
Review Comment:
Follow-up: since https://github.com/apache/arrow-rs/pull/11237 merged,
sorted order is an invariant of `PushBuffers`. Lookups in `find` use a binary
search and depend on it, so `release_ranges` must keep it. After the rebase the
tests check it with `assert_invariants` and do not assert the exact internal
order.
##########
parquet/src/util/push_buffers.rs:
##########
@@ -253,6 +399,126 @@ impl ChunkReader for PushBuffers {
mod tests {
use super::*;
+ /// Release one range.
+ #[cfg(feature = "arrow")]
+ fn release(buffers: &mut PushBuffers, range: Range<u64>) {
+ buffers.release_ranges(std::slice::from_ref(&range));
+ }
+
+ #[test]
+ #[cfg(feature = "arrow")]
+ fn release_ranges_without_overlap_keeps_buffers() {
+ let mut buffers = PushBuffers::new(100);
+ buffers
+ .push_range(20..24, Bytes::from_static(b"abcd"))
+ .unwrap();
+ buffers
+ .push_range(0..10, Bytes::from_static(b"0123456789"))
+ .unwrap();
+
+ release(&mut buffers, 90..95);
+ release(&mut buffers, 10..20);
+ assert_eq!(buffers.ranges, vec![20..24, 0..10]);
+ assert_eq!(buffers.buffered_bytes(), 14);
+
+ // A release that overlaps a buffer sorts the buffers.
+ release(&mut buffers, 22..24);
Review Comment:
Follow-up: since https://github.com/apache/arrow-rs/pull/11237 merged,
sorted order is an invariant of `PushBuffers`. Lookups in `find` use a binary
search and depend on it, so `release_ranges` must keep it. After the rebase the
tests check it with `assert_invariants` and do not assert the exact internal
order.
--
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]
