This is an automated email from the ASF dual-hosted git repository.
agrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new d8ab910e fix: Optimize decimal creation macros (#764)
d8ab910e is described below
commit d8ab910e6933be1136a5242d3d08d1103743b04e
Author: KAZUYUKI TANIMURA <[email protected]>
AuthorDate: Tue Aug 6 06:18:11 2024 -0700
fix: Optimize decimal creation macros (#764)
---
native/core/benches/parquet_decode.rs | 10 ++-
native/core/src/parquet/read/values.rs | 149 ++++++---------------------------
2 files changed, 34 insertions(+), 125 deletions(-)
diff --git a/native/core/benches/parquet_decode.rs
b/native/core/benches/parquet_decode.rs
index 7176bf9c..75eefd5d 100644
--- a/native/core/benches/parquet_decode.rs
+++ b/native/core/benches/parquet_decode.rs
@@ -16,12 +16,12 @@
// under the License.
use arrow_buffer::ToByteSlice;
-use comet::parquet::read::values::{copy_i32_to_i16, copy_i32_to_u16};
+use comet::parquet::read::values::{copy_i32_to_i16, copy_i32_to_u16,
copy_i64_to_i64};
use criterion::{criterion_group, criterion_main, Criterion};
fn criterion_benchmark(c: &mut Criterion) {
let num = 1000;
- let source = vec![78_i8; num * 4];
+ let source = vec![78_i8; num * 8];
let mut group = c.benchmark_group("parquet_decode");
group.bench_function("decode_i32_to_i16", |b| {
let mut dest: Vec<u8> = vec![b' '; num * 2];
@@ -35,6 +35,12 @@ fn criterion_benchmark(c: &mut Criterion) {
copy_i32_to_u16(source.to_byte_slice(), dest.as_mut_slice(), num);
});
});
+ group.bench_function("decode_i64_to_i64", |b| {
+ let mut dest: Vec<u8> = vec![b' '; num * 8];
+ b.iter(|| {
+ copy_i64_to_i64(source.to_byte_slice(), dest.as_mut_slice(), num);
+ });
+ });
}
// Create UTF8 batch with strings representing ints, floats, nulls
diff --git a/native/core/src/parquet/read/values.rs
b/native/core/src/parquet/read/values.rs
index b89face0..324f6106 100644
--- a/native/core/src/parquet/read/values.rs
+++ b/native/core/src/parquet/read/values.rs
@@ -27,7 +27,7 @@ use crate::write_null;
use crate::write_val_or_null;
use crate::{
common::bit::{self, BitReader},
- parquet::{data_type::*, read::DECIMAL_BYTE_WIDTH, ParquetMutableVector},
+ parquet::{data_type::*, ParquetMutableVector},
unlikely,
};
use arrow::datatypes::DataType as ArrowDataType;
@@ -214,6 +214,9 @@ make_int_variant_dict_impl!(Int32ToDecimal64Type, i32, i64);
make_int_variant_dict_impl!(Int32ToDoubleType, i32, f64);
make_int_variant_dict_impl!(Int32TimestampMicrosType, i32, i64);
make_int_variant_dict_impl!(FloatToDoubleType, f32, f64);
+make_int_variant_dict_impl!(Int32DecimalType, i128, i128);
+make_int_variant_dict_impl!(Int64DecimalType, i128, i128);
+make_int_variant_dict_impl!(UInt64Type, u128, u128);
impl PlainDecoding for Int32DateType {
fn decode(src: &mut PlainDecoderInner, dst: &mut ParquetMutableVector,
num: usize) {
@@ -483,7 +486,7 @@ macro_rules! make_int_decimal_variant_impl {
impl PlainDecoding for $ty {
fn decode(src: &mut PlainDecoderInner, dst: &mut
ParquetMutableVector, num: usize) {
let dst_slice = dst.value_buffer.as_slice_mut();
- let dst_offset = dst.num_values * 8;
+ let dst_offset = dst.num_values *
std::mem::size_of::<$dst_type>();
$copy_fn(&src.data[src.offset..], &mut
dst_slice[dst_offset..], num);
let src_precision = src.desc.type_precision() as u32;
@@ -532,7 +535,10 @@ macro_rules! make_int_decimal_variant_impl {
};
}
make_int_decimal_variant_impl!(Int32ToDecimal64Type, copy_i32_to_i64, 4, i64);
+make_int_decimal_variant_impl!(Int32DecimalType, copy_i32_to_i128, 4, i128);
make_int_decimal_variant_impl!(Int64ToDecimal64Type, copy_i64_to_i64, 8, i64);
+make_int_decimal_variant_impl!(Int64DecimalType, copy_i64_to_i128, 8, i128);
+make_int_decimal_variant_impl!(UInt64Type, copy_u64_to_u128, 8, u128);
#[macro_export]
macro_rules! write_val_or_null {
@@ -612,22 +618,13 @@ macro_rules! generate_cast_to_signed {
generate_cast_to_signed!(copy_i32_to_i8, i32, i8);
generate_cast_to_signed!(copy_i32_to_i16, i32, i16);
generate_cast_to_signed!(copy_i32_to_i64, i32, i64);
+generate_cast_to_signed!(copy_i32_to_i128, i32, i128);
generate_cast_to_signed!(copy_i32_to_f64, i32, f64);
+generate_cast_to_signed!(copy_i64_to_i64, i64, i64);
+generate_cast_to_signed!(copy_i64_to_i128, i64, i128);
+generate_cast_to_signed!(copy_u64_to_u128, u64, u128);
generate_cast_to_signed!(copy_f32_to_f64, f32, f64);
-fn copy_i64_to_i64(src: &[u8], dst: &mut [u8], num: usize) {
- debug_assert!(
- src.len() >= num * std::mem::size_of::<i64>(),
- "Source slice is too small"
- );
- debug_assert!(
- dst.len() >= num * std::mem::size_of::<i64>(),
- "Destination slice is too small"
- );
-
- bit::memcpy_value(src, std::mem::size_of::<i64>() * num, dst);
-}
-
// Shared implementation for variants of Binary type
macro_rules! make_plain_binary_impl {
($($ty: ident), *) => {
@@ -758,112 +755,17 @@ macro_rules! make_plain_dict_binary_impl {
make_plain_dict_binary_impl! { ByteArrayType, StringType }
-macro_rules! make_plain_decimal_impl {
- ($is_signed: expr, $($ty: ident; $need_convert: expr), *) => {
- $(
- impl PlainDecoding for $ty {
- fn decode(src: &mut PlainDecoderInner, dst: &mut
ParquetMutableVector, num: usize) {
- let byte_width = src.desc.type_length() as usize;
-
- let src_data = &src.data[src.offset..];
- let dst_data = &mut dst.value_buffer[dst.num_values *
DECIMAL_BYTE_WIDTH..];
-
- let mut src_offset = 0;
- let mut dst_offset = 0;
-
- debug_assert!(byte_width <= DECIMAL_BYTE_WIDTH);
-
- let src_precision = src.desc.type_precision() as u32;
- let src_scale = ::std::cmp::max(src.desc.type_scale(), 0)
as u32;
- let (dst_precision, dst_scale) = match dst.arrow_type {
- ArrowDataType::Decimal128(p, s) if s >= 0 => (p as
u32, s as u32),
- _ => unreachable!(),
- };
- let upper = 10_i128.pow(dst_precision);
- let mul_div = 10_i128.pow(dst_scale.abs_diff(src_scale));
-
- for i in 0..num {
- let s = &mut dst_data[dst_offset..];
-
- bit::memcpy(
- &src_data[src_offset..src_offset + byte_width],
- s,
- );
-
- // Swap the order of bytes to make it little-endian.
- if $need_convert {
- for i in 0..byte_width / 2 {
- s.swap(i, byte_width - i - 1);
- }
- }
-
- if $is_signed {
- // Check if the most significant bit is 1
(negative in 2's complement).
- // If so, also fill pad the remaining bytes with
0xff.
- if s[byte_width - 1] & 0x80 == 0x80 {
- s[byte_width..DECIMAL_BYTE_WIDTH].fill(0xff);
- }
- }
-
- if dst_scale > src_scale {
- let v = s.as_mut_ptr() as *mut i128;
- unsafe {
- // SAFETY the caller must ensure `i`th pointer
is in bounds
- write_val_or_null!(v, v.read_unaligned() *
mul_div, upper, dst, i);
- }
- } else if dst_scale < src_scale {
- let v = s.as_mut_ptr() as *mut i128;
- unsafe {
- // SAFETY the caller must ensure `i`th pointer
is in bounds
- write_val_or_null!(v, v.read_unaligned() /
mul_div, upper, dst, i);
- }
- } else if src_precision > dst_precision {
- let v = s.as_mut_ptr() as *mut i128;
- write_null!(unsafe { v.read_unaligned() }, upper,
dst, i);
- }
-
- src_offset += byte_width;
- dst_offset += DECIMAL_BYTE_WIDTH;
- }
-
- src.offset += num * byte_width;
- }
-
- #[inline]
- fn skip(src: &mut PlainDecoderInner, num: usize) {
- let num_bytes_to_skip = num * src.desc.type_length() as
usize;
- src.offset += num_bytes_to_skip;
- }
- }
-
- impl PlainDictDecoding for $ty {
- fn decode_dict_one(_: usize, val_idx: usize, src:
&ParquetMutableVector, dst: &mut ParquetMutableVector, _: usize) {
- let src_offset = val_idx * DECIMAL_BYTE_WIDTH;
- let dst_offset = dst.num_values * DECIMAL_BYTE_WIDTH;
-
- bit::memcpy(
- &src.value_buffer[src_offset..src_offset +
DECIMAL_BYTE_WIDTH],
- &mut dst.value_buffer[dst_offset..dst_offset +
DECIMAL_BYTE_WIDTH],
- );
- }
- }
- )*
- }
-}
-
-make_plain_decimal_impl!(true, Int32DecimalType; false, Int64DecimalType;
false, FLBADecimalType; true);
-make_plain_decimal_impl!(false, UInt64Type; false);
-
macro_rules! make_plain_decimal_int_impl {
- ($($ty: ident; $num_bytes: expr), *) => {
+ ($($ty: ident; $dst_type:ty), *) => {
$(
impl PlainDecoding for $ty {
fn decode(src: &mut PlainDecoderInner, dst: &mut
ParquetMutableVector, num: usize) {
+ let num_bytes = std::mem::size_of::<$dst_type>();
let byte_width = src.desc.type_length() as usize;
- let num_bits = 64.min(8 * byte_width);
+ let num_bits = num_bytes.saturating_sub(byte_width) * 8;
let src_data = &src.data[src.offset..];
- let dst_data = &mut dst.value_buffer[dst.num_values *
$num_bytes..];
+ let dst_data = &mut dst.value_buffer[dst.num_values *
num_bytes..];
let mut src_offset = 0;
@@ -873,22 +775,22 @@ macro_rules! make_plain_decimal_int_impl {
ArrowDataType::Decimal128(p, s) if s >= 0 => (p as
u32, s as u32),
_ => (src_precision, src_scale),
};
- let upper = 10_i64.pow(dst_precision);
- let mul_div = 10_i64.pow(dst_scale.abs_diff(src_scale));
+ let upper = (10 as $dst_type).pow(dst_precision);
+ let mul_div = (10 as
$dst_type).pow(dst_scale.abs_diff(src_scale));
for i in 0..num {
- let mut unscaled: i64 = 0;
+ let mut unscaled: $dst_type = 0;
for _ in 0..byte_width {
- unscaled = unscaled << 8 | src_data[src_offset] as
i64;
+ unscaled = unscaled << 8 | src_data[src_offset] as
$dst_type;
src_offset += 1;
}
- unscaled = (unscaled << (64 - num_bits)) >> (64 -
num_bits);
+ unscaled = (unscaled << num_bits) >> num_bits;
if dst_scale > src_scale {
unscaled *= mul_div;
} else if dst_scale < src_scale {
unscaled /= mul_div;
}
- bit::memcpy_value(&unscaled, $num_bytes, &mut
dst_data[i * $num_bytes..]);
+ bit::memcpy_value(&unscaled, num_bytes, &mut
dst_data[i * num_bytes..]);
if src_precision > dst_precision {
write_null!(unscaled, upper, dst, i);
}
@@ -906,9 +808,10 @@ macro_rules! make_plain_decimal_int_impl {
impl PlainDictDecoding for $ty {
#[inline]
fn decode_dict_one(_: usize, val_idx: usize, src:
&ParquetMutableVector, dst: &mut ParquetMutableVector, _: usize) {
+ let num_bytes = std::mem::size_of::<$dst_type>();
bit::memcpy(
- &src.value_buffer[val_idx * $num_bytes..(val_idx + 1)
* $num_bytes],
- &mut dst.value_buffer[dst.num_values * $num_bytes..],
+ &src.value_buffer[val_idx * num_bytes..(val_idx + 1) *
num_bytes],
+ &mut dst.value_buffer[dst.num_values * num_bytes..],
);
}
}
@@ -916,7 +819,7 @@ macro_rules! make_plain_decimal_int_impl {
};
}
-make_plain_decimal_int_impl!(FLBADecimal32Type; 4, FLBADecimal64Type; 8);
+make_plain_decimal_int_impl!(FLBADecimal32Type; i32, FLBADecimal64Type; i64,
FLBADecimalType; i128);
// Int96 contains 12 bytes
const INT96_SRC_BYTE_WIDTH: usize = 12;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
