villebro commented on code in PR #2522:
URL:
https://github.com/apache/datafusion-ballista/pull/2522#discussion_r4188043524
##########
ballista/core/src/execution_plans/sort_shuffle/partitioned_batch_iterator.rs:
##########
@@ -18,30 +18,47 @@
//! Iterator that materializes per-partition rows into well-sized
//! `RecordBatch`es using `arrow::compute::interleave_record_batch`.
-use datafusion::arrow::array::{Array, ArrayRef, BinaryViewArray,
StringViewArray};
+use datafusion::arrow::array::{
+ Array, ArrayRef, BinaryViewArray, GenericByteViewArray, StringViewArray,
+};
use datafusion::arrow::compute::interleave_record_batch;
+use datafusion::arrow::datatypes::ByteViewType;
use datafusion::arrow::record_batch::RecordBatch;
use datafusion::common::DataFusionError;
use datafusion::error::Result;
use std::sync::Arc;
-/// Compacts `Utf8View` / `BinaryView` columns by running `gc()` on them.
+/// Returns true when a view array's data buffers are more than twice the
+/// bytes its views actually reference, the same threshold arrow's
+/// `BatchCoalescer` uses before copying strings. Arrays with no data buffers
+/// (all values inlined) never need compaction.
+fn needs_gc<T: ByteViewType + ?Sized>(array: &GenericByteViewArray<T>) -> bool
{
+ let actual: usize = array.data_buffers().iter().map(|b| b.len()).sum();
+ actual > 2 * array.total_buffer_bytes_used()
+}
Review Comment:
I don't feel competent to say Arrow has the wrong design in
`BatchCoalescer`, but is the 2x heuristic potentially too simplistic in a case
where the array is very large? For a smaller buffers I think this sounds
reasonable, but as the buffer gets bigger, the case for compacting it grows
even for more insignificant percentage gains.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
