Skip to content

Commit 99a6300

Browse files
jorgecarleitaokszucs
jorgecarleitao
authored and
kszucs
committed
ARROW-11108: [Rust] Fixed performance issue in mutableBuffer.
This PR refactors `MutableBuffer::extend_from_slice` to remove the need to use `to_byte_slice` on every call, thereby removing its level of indirection, that does not allow the compiler to optimize out some code. This is the second performance improvement originally presented in #8796 and, together with #9027 , brings the performance of "MutableBuffer" to the same level as `Vec<u8>`, in particular to building buffers on the fly. Basically, when converting to a byte slice `&[u8]`, the compiler loses the type size information, and thus needs to perform extra checks and can't just optimize out the code. This PR adopts the same API as `Vec<T>::extend_from_slice`, but since our buffers are in `u8` (i.e. a la `Vec<u8>`), I made the signature ``` pub fn extend_from_slice<T: ToByteSlice>(&mut self, items: &[T]) pub fn push<T: ToByteSlice>(&mut self, item: &T) ``` i.e. it consumes something that can be converted to a byte slice, but internally makes the conversion to bytes (as `to_byte_slice` was doing). Credits for the root cause analysis that lead to this PR go to @Dandandan, [originally fielded here](#9016 (comment)). > [...] current conversion to a byte slice may add some overhead? - @Dandandan Benches (against master, so, both this PR and #9044 ): ``` Switched to branch 'perf_buffer' Your branch and 'origin/perf_buffer' have diverged, and have 6 and 1 different commits each, respectively. (use "git pull" to merge the remote branch into yours) Compiling arrow v3.0.0-SNAPSHOT (/Users/jorgecarleitao/projects/arrow/rust/arrow) Finished bench [optimized] target(s) in 1m 00s Running /Users/jorgecarleitao/projects/arrow/rust/target/release/deps/buffer_create-915da5f1abaf0471 Gnuplot not found, using plotters backend mutable time: [463.11 us 463.57 us 464.07 us] change: [-19.508% -18.571% -17.526%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 1 (1.00%) high mild 9 (9.00%) high severe mutable prepared time: [527.84 us 528.46 us 529.14 us] change: [-13.356% -12.522% -11.790%] (p = 0.00 < 0.05) Performance has improved. Found 12 outliers among 100 measurements (12.00%) 5 (5.00%) high mild 7 (7.00%) high severe Benchmarking from_slice: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.1s, enable flat sampling, or reduce sample count to 60. from_slice time: [1.1968 ms 1.1979 ms 1.1991 ms] change: [-6.8697% -6.2029% -5.5812%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 3 (3.00%) high mild 7 (7.00%) high severe from_slice prepared time: [917.49 us 918.89 us 920.60 us] change: [-6.5111% -5.9102% -5.3038%] (p = 0.00 < 0.05) Performance has improved. Found 10 outliers among 100 measurements (10.00%) 4 (4.00%) high mild 6 (6.00%) high severe ``` Closes #9076 from jorgecarleitao/perf_buffer Authored-by: Jorge C. Leitao <jorgecarleitao@gmail.com> Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>
1 parent aeeeca0 commit 99a6300

38 files changed

+492
-552
lines changed

‎rust/arrow/benches/buffer_create.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ fn mutable_buffer(data: &[Vec<u32>], capacity: usize) -> Buffer {
3333
criterion::black_box({
3434
let mut result = MutableBuffer::new(capacity);
3535

36-
data.iter()
37-
.for_each(|vec| result.extend_from_slice(vec.to_byte_slice()));
36+
data.iter().for_each(|vec| result.extend_from_slice(vec));
3837

3938
result.into()
4039
})

‎rust/arrow/src/array/array_binary.rs‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ use super::{
2727
array::print_long_array, raw_pointer::RawPtrBox, Array, ArrayData, ArrayDataRef,
2828
FixedSizeListArray, GenericBinaryIter, GenericListArray, OffsetSizeTrait,
2929
};
30+
use crate::buffer::Buffer;
3031
use crate::util::bit_util;
31-
use crate::{buffer::Buffer, datatypes::ToByteSlice};
3232
use crate::{buffer::MutableBuffer, datatypes::DataType};
3333

3434
/// Like OffsetSizeTrait, but specialized for Binary
@@ -110,8 +110,8 @@ impl<OffsetSize: BinaryOffsetSizeTrait> GenericBinaryArray<OffsetSize> {
110110
}
111111
let array_data = ArrayData::builder(OffsetSize::DATA_TYPE)
112112
.len(v.len())
113-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
114-
.add_buffer(Buffer::from(&values[..]))
113+
.add_buffer(Buffer::from_slice_ref(&offsets))
114+
.add_buffer(Buffer::from_slice_ref(&values))
115115
.build();
116116
GenericBinaryArray::<OffsetSize>::from(array_data)
117117
}
@@ -245,8 +245,8 @@ where
245245

246246
let array_data = ArrayData::builder(OffsetSize::DATA_TYPE)
247247
.len(data_len)
248-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
249-
.add_buffer(Buffer::from(&values[..]))
248+
.add_buffer(Buffer::from_slice_ref(&offsets))
249+
.add_buffer(Buffer::from_slice_ref(&values))
250250
.null_bit_buffer(null_buf.into())
251251
.build();
252252
Self::from(array_data)
@@ -368,7 +368,7 @@ impl From<Vec<Option<Vec<u8>>>> for FixedSizeBinaryArray {
368368
.all(|item| item.len() == size));
369369

370370
let num_bytes = bit_util::ceil(len, 8);
371-
let mut null_buf = MutableBuffer::new(num_bytes).with_bitset(num_bytes, false);
371+
let mut null_buf = MutableBuffer::from_len_zeroed(num_bytes);
372372
let null_slice = null_buf.as_slice_mut();
373373

374374
data.iter().enumerate().for_each(|(i, entry)| {
@@ -641,8 +641,8 @@ mod tests {
641641
// Array data: ["hello", "", "parquet"]
642642
let array_data = ArrayData::builder(DataType::Binary)
643643
.len(3)
644-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
645-
.add_buffer(Buffer::from(&values[..]))
644+
.add_buffer(Buffer::from_slice_ref(&offsets))
645+
.add_buffer(Buffer::from_slice_ref(&values))
646646
.build();
647647
let binary_array = BinaryArray::from(array_data);
648648
assert_eq!(3, binary_array.len());
@@ -664,8 +664,8 @@ mod tests {
664664
let array_data = ArrayData::builder(DataType::Binary)
665665
.len(4)
666666
.offset(1)
667-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
668-
.add_buffer(Buffer::from(&values[..]))
667+
.add_buffer(Buffer::from_slice_ref(&offsets))
668+
.add_buffer(Buffer::from_slice_ref(&values))
669669
.build();
670670
let binary_array = BinaryArray::from(array_data);
671671
assert_eq!(
@@ -688,8 +688,8 @@ mod tests {
688688
// Array data: ["hello", "", "parquet"]
689689
let array_data = ArrayData::builder(DataType::LargeBinary)
690690
.len(3)
691-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
692-
.add_buffer(Buffer::from(&values[..]))
691+
.add_buffer(Buffer::from_slice_ref(&offsets))
692+
.add_buffer(Buffer::from_slice_ref(&values))
693693
.build();
694694
let binary_array = LargeBinaryArray::from(array_data);
695695
assert_eq!(3, binary_array.len());
@@ -711,8 +711,8 @@ mod tests {
711711
let array_data = ArrayData::builder(DataType::LargeBinary)
712712
.len(4)
713713
.offset(1)
714-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
715-
.add_buffer(Buffer::from(&values[..]))
714+
.add_buffer(Buffer::from_slice_ref(&offsets))
715+
.add_buffer(Buffer::from_slice_ref(&values))
716716
.build();
717717
let binary_array = LargeBinaryArray::from(array_data);
718718
assert_eq!(
@@ -739,14 +739,14 @@ mod tests {
739739
// Array data: ["hello", "", "parquet"]
740740
let array_data1 = ArrayData::builder(DataType::Binary)
741741
.len(3)
742-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
743-
.add_buffer(Buffer::from(&values[..]))
742+
.add_buffer(Buffer::from_slice_ref(&offsets))
743+
.add_buffer(Buffer::from_slice_ref(&values))
744744
.build();
745745
let binary_array1 = BinaryArray::from(array_data1);
746746

747747
let array_data2 = ArrayData::builder(DataType::Binary)
748748
.len(3)
749-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
749+
.add_buffer(Buffer::from_slice_ref(&offsets))
750750
.add_child_data(values_data)
751751
.build();
752752
let list_array = ListArray::from(array_data2);
@@ -778,14 +778,14 @@ mod tests {
778778
// Array data: ["hello", "", "parquet"]
779779
let array_data1 = ArrayData::builder(DataType::LargeBinary)
780780
.len(3)
781-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
782-
.add_buffer(Buffer::from(&values[..]))
781+
.add_buffer(Buffer::from_slice_ref(&offsets))
782+
.add_buffer(Buffer::from_slice_ref(&values))
783783
.build();
784784
let binary_array1 = LargeBinaryArray::from(array_data1);
785785

786786
let array_data2 = ArrayData::builder(DataType::Binary)
787787
.len(3)
788-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
788+
.add_buffer(Buffer::from_slice_ref(&offsets))
789789
.add_child_data(values_data)
790790
.build();
791791
let list_array = LargeListArray::from(array_data2);
@@ -838,13 +838,13 @@ mod tests {
838838
let values: [u32; 12] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
839839
let values_data = ArrayData::builder(DataType::UInt32)
840840
.len(12)
841-
.add_buffer(Buffer::from(values[..].to_byte_slice()))
841+
.add_buffer(Buffer::from_slice_ref(&values))
842842
.build();
843843
let offsets: [i32; 4] = [0, 5, 5, 12];
844844

845845
let array_data = ArrayData::builder(DataType::Utf8)
846846
.len(3)
847-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
847+
.add_buffer(Buffer::from_slice_ref(&offsets))
848848
.add_child_data(values_data)
849849
.build();
850850
let list_array = ListArray::from(array_data);
@@ -860,14 +860,14 @@ mod tests {
860860
let values: [u32; 12] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
861861
let values_data = ArrayData::builder(DataType::UInt32)
862862
.len(12)
863-
.add_buffer(Buffer::from(values[..].to_byte_slice()))
863+
.add_buffer(Buffer::from_slice_ref(&values))
864864
.add_child_data(ArrayData::builder(DataType::Boolean).build())
865865
.build();
866866
let offsets: [i32; 4] = [0, 5, 5, 12];
867867

868868
let array_data = ArrayData::builder(DataType::Utf8)
869869
.len(3)
870-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
870+
.add_buffer(Buffer::from_slice_ref(&offsets))
871871
.add_child_data(values_data)
872872
.build();
873873
let list_array = ListArray::from(array_data);
@@ -934,7 +934,7 @@ mod tests {
934934
let values: [u32; 12] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
935935
let values_data = ArrayData::builder(DataType::UInt32)
936936
.len(12)
937-
.add_buffer(Buffer::from(values[..].to_byte_slice()))
937+
.add_buffer(Buffer::from_slice_ref(&values))
938938
.add_child_data(ArrayData::builder(DataType::Boolean).build())
939939
.build();
940940

@@ -957,8 +957,8 @@ mod tests {
957957
let offsets: [i32; 4] = [0, 5, 5, 12];
958958
let array_data = ArrayData::builder(DataType::Binary)
959959
.len(3)
960-
.add_buffer(Buffer::from(offsets.to_byte_slice()))
961-
.add_buffer(Buffer::from(&values[..]))
960+
.add_buffer(Buffer::from_slice_ref(&offsets))
961+
.add_buffer(Buffer::from_slice_ref(&values))
962962
.build();
963963
let binary_array = BinaryArray::from(array_data);
964964
binary_array.value(4);

‎rust/arrow/src/array/array_boolean.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ impl<Ptr: Borrow<Option<bool>>> FromIterator<Ptr> for BooleanArray {
163163
let data_len = data_len.expect("Iterator must be sized"); // panic if no upper bound.
164164

165165
let num_bytes = bit_util::ceil(data_len, 8);
166-
let mut null_buf = MutableBuffer::new(num_bytes).with_bitset(num_bytes, false);
167-
let mut val_buf = MutableBuffer::new(num_bytes).with_bitset(num_bytes, false);
166+
let mut null_buf = MutableBuffer::from_len_zeroed(num_bytes);
167+
let mut val_buf = MutableBuffer::from_len_zeroed(num_bytes);
168168

169169
let data = val_buf.as_slice_mut();
170170

‎rust/arrow/src/array/array_list.rs‎

Lines changed: 16 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,7 @@ impl fmt::Debug for FixedSizeListArray {
298298
#[cfg(test)]
299299
mod tests {
300300
use crate::{
301-
array::ArrayData,
302-
array::Int32Array,
303-
buffer::Buffer,
304-
datatypes::{Field, ToByteSlice},
305-
memory,
301+
array::ArrayData, array::Int32Array, buffer::Buffer, datatypes::Field,
306302
util::bit_util,
307303
};
308304

@@ -313,12 +309,12 @@ mod tests {
313309
// Construct a value array
314310
let value_data = ArrayData::builder(DataType::Int32)
315311
.len(8)
316-
.add_buffer(Buffer::from(&[0, 1, 2, 3, 4, 5, 6, 7].to_byte_slice()))
312+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7]))
317313
.build();
318314

319315
// Construct a buffer for value offsets, for the nested array:
320316
// [[0, 1, 2], [3, 4, 5], [6, 7]]
321-
let value_offsets = Buffer::from(&[0, 3, 6, 8].to_byte_slice());
317+
let value_offsets = Buffer::from_slice_ref(&[0, 3, 6, 8]);
322318

323319
// Construct a list array from the above two
324320
let list_data_type =
@@ -383,12 +379,12 @@ mod tests {
383379
// Construct a value array
384380
let value_data = ArrayData::builder(DataType::Int32)
385381
.len(8)
386-
.add_buffer(Buffer::from(&[0, 1, 2, 3, 4, 5, 6, 7].to_byte_slice()))
382+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7]))
387383
.build();
388384

389385
// Construct a buffer for value offsets, for the nested array:
390386
// [[0, 1, 2], [3, 4, 5], [6, 7]]
391-
let value_offsets = Buffer::from(&[0i64, 3, 6, 8].to_byte_slice());
387+
let value_offsets = Buffer::from_slice_ref(&[0i64, 3, 6, 8]);
392388

393389
// Construct a list array from the above two
394390
let list_data_type =
@@ -453,7 +449,7 @@ mod tests {
453449
// Construct a value array
454450
let value_data = ArrayData::builder(DataType::Int32)
455451
.len(9)
456-
.add_buffer(Buffer::from(&[0, 1, 2, 3, 4, 5, 6, 7, 8].to_byte_slice()))
452+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7, 8]))
457453
.build();
458454

459455
// Construct a list array from the above two
@@ -522,7 +518,7 @@ mod tests {
522518
// Construct a value array
523519
let value_data = ArrayData::builder(DataType::Int32)
524520
.len(8)
525-
.add_buffer(Buffer::from(&[0, 1, 2, 3, 4, 5, 6, 7].to_byte_slice()))
521+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7]))
526522
.build();
527523

528524
// Construct a list array from the above two
@@ -542,15 +538,12 @@ mod tests {
542538
// Construct a value array
543539
let value_data = ArrayData::builder(DataType::Int32)
544540
.len(10)
545-
.add_buffer(Buffer::from(
546-
&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9].to_byte_slice(),
547-
))
541+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]))
548542
.build();
549543

550544
// Construct a buffer for value offsets, for the nested array:
551545
// [[0, 1], null, null, [2, 3], [4, 5], null, [6, 7, 8], null, [9]]
552-
let value_offsets =
553-
Buffer::from(&[0, 2, 2, 2, 4, 6, 6, 9, 9, 10].to_byte_slice());
546+
let value_offsets = Buffer::from_slice_ref(&[0, 2, 2, 2, 4, 6, 6, 9, 9, 10]);
554547
// 01011001 00000001
555548
let mut null_bits: [u8; 2] = [0; 2];
556549
bit_util::set_bit(&mut null_bits, 0);
@@ -607,15 +600,12 @@ mod tests {
607600
// Construct a value array
608601
let value_data = ArrayData::builder(DataType::Int32)
609602
.len(10)
610-
.add_buffer(Buffer::from(
611-
&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9].to_byte_slice(),
612-
))
603+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]))
613604
.build();
614605

615606
// Construct a buffer for value offsets, for the nested array:
616607
// [[0, 1], null, null, [2, 3], [4, 5], null, [6, 7, 8], null, [9]]
617-
let value_offsets =
618-
Buffer::from(&[0i64, 2, 2, 2, 4, 6, 6, 9, 9, 10].to_byte_slice());
608+
let value_offsets = Buffer::from_slice_ref(&[0i64, 2, 2, 2, 4, 6, 6, 9, 9, 10]);
619609
// 01011001 00000001
620610
let mut null_bits: [u8; 2] = [0; 2];
621611
bit_util::set_bit(&mut null_bits, 0);
@@ -674,9 +664,7 @@ mod tests {
674664
// Construct a value array
675665
let value_data = ArrayData::builder(DataType::Int32)
676666
.len(10)
677-
.add_buffer(Buffer::from(
678-
&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9].to_byte_slice(),
679-
))
667+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]))
680668
.build();
681669

682670
// Set null buts for the nested array:
@@ -737,7 +725,7 @@ mod tests {
737725
fn test_list_array_invalid_buffer_len() {
738726
let value_data = ArrayData::builder(DataType::Int32)
739727
.len(8)
740-
.add_buffer(Buffer::from(&[0, 1, 2, 3, 4, 5, 6, 7].to_byte_slice()))
728+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7]))
741729
.build();
742730
let list_data_type =
743731
DataType::List(Box::new(Field::new("item", DataType::Int32, false)));
@@ -753,7 +741,7 @@ mod tests {
753741
expected = "ListArray should contain a single child array (values array)"
754742
)]
755743
fn test_list_array_invalid_child_array_len() {
756-
let value_offsets = Buffer::from(&[0, 2, 5, 7].to_byte_slice());
744+
let value_offsets = Buffer::from_slice_ref(&[0, 2, 5, 7]);
757745
let list_data_type =
758746
DataType::List(Box::new(Field::new("item", DataType::Int32, false)));
759747
let list_data = ArrayData::builder(list_data_type)
@@ -768,10 +756,10 @@ mod tests {
768756
fn test_list_array_invalid_value_offset_start() {
769757
let value_data = ArrayData::builder(DataType::Int32)
770758
.len(8)
771-
.add_buffer(Buffer::from(&[0, 1, 2, 3, 4, 5, 6, 7].to_byte_slice()))
759+
.add_buffer(Buffer::from_slice_ref(&[0, 1, 2, 3, 4, 5, 6, 7]))
772760
.build();
773761

774-
let value_offsets = Buffer::from(&[2, 2, 5, 7].to_byte_slice());
762+
let value_offsets = Buffer::from_slice_ref(&[2, 2, 5, 7]);
775763

776764
let list_data_type =
777765
DataType::List(Box::new(Field::new("item", DataType::Int32, false)));
@@ -782,35 +770,4 @@ mod tests {
782770
.build();
783771
ListArray::from(list_data);
784772
}
785-
786-
#[test]
787-
#[should_panic(expected = "memory is not aligned")]
788-
fn test_primitive_array_alignment() {
789-
let ptr = memory::allocate_aligned(8);
790-
let buf = unsafe { Buffer::from_raw_parts(ptr, 8, 8) };
791-
let buf2 = buf.slice(1);
792-
let array_data = ArrayData::builder(DataType::Int32).add_buffer(buf2).build();
793-
Int32Array::from(array_data);
794-
}
795-
796-
#[test]
797-
#[should_panic(expected = "memory is not aligned")]
798-
fn test_list_array_alignment() {
799-
let ptr = memory::allocate_aligned(8);
800-
let buf = unsafe { Buffer::from_raw_parts(ptr, 8, 8) };
801-
let buf2 = buf.slice(1);
802-
803-
let values: [i32; 8] = [0; 8];
804-
let value_data = ArrayData::builder(DataType::Int32)
805-
.add_buffer(Buffer::from(values.to_byte_slice()))
806-
.build();
807-
808-
let list_data_type =
809-
DataType::List(Box::new(Field::new("item", DataType::Int32, false)));
810-
let list_data = ArrayData::builder(list_data_type)
811-
.add_buffer(buf2)
812-
.add_child_data(value_data)
813-
.build();
814-
ListArray::from(list_data);
815-
}
816773
}

0 commit comments

Comments
 (0)