Skip to content

Commit 1fa53f1

Browse files
committed
apacheGH-43891: [C++][Parquet] Faster reading of FIXED_LEN_BYTE_ARRAY data
Improve the performance of reading FIXED_LEN_BYTE_ARRAY columns to Arrow, by avoiding an intermediate read to FLBA structures. This especially helps improve the speed of reading FLOAT16 columns and makes it faster than FLOAT.
1 parent f8cd17c commit 1fa53f1

9 files changed

Lines changed: 569 additions & 401 deletions

File tree

cpp/src/arrow/array/builder_binary.h

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -855,11 +855,41 @@ class ARROW_EXPORT FixedSizeBinaryBuilder : public ArrayBuilder {
855855
/// This pointer becomes invalid on the next modifying operation.
856856
const uint8_t* GetValue(int64_t i) const;
857857

858-
/// Temporary access to a value.
858+
/// Temporary mutable access to a value.
859+
///
860+
/// This pointer becomes invalid on the next modifying operation.
861+
uint8_t* GetMutableValue(int64_t i) {
862+
uint8_t* data_ptr = byte_builder_.mutable_data();
863+
return data_ptr + i * byte_width_;
864+
}
865+
866+
/// Temporary mutable access to a value.
859867
///
860868
/// This view becomes invalid on the next modifying operation.
861869
std::string_view GetView(int64_t i) const;
862870

871+
/// Advance builder without allocating nor writing any values
872+
///
873+
/// The internal pointer is advanced by `length` values and the same number
874+
/// of non-null entries are appended to the validity bitmap.
875+
/// This method assumes that the `length` values were populated directly,
876+
/// for example using `GetMutableValue`.
877+
void UnsafeAdvance(int64_t length) {
878+
byte_builder_.UnsafeAdvance(length * byte_width_);
879+
UnsafeAppendToBitmap(length, true);
880+
}
881+
882+
/// Advance builder without allocating nor writing any values
883+
///
884+
/// The internal pointer is advanced by `length` values and the same number
885+
/// of validity bits are appended to the validity bitmap.
886+
/// This method assumes that the `length` values were populated directly,
887+
/// for example using `GetMutableValue`.
888+
void UnsafeAdvance(int64_t length, const uint8_t* validity, int64_t valid_bits_offset) {
889+
byte_builder_.UnsafeAdvance(length * byte_width_);
890+
UnsafeAppendToBitmap(validity, valid_bits_offset, length);
891+
}
892+
863893
static constexpr int64_t memory_limit() {
864894
return std::numeric_limits<int64_t>::max() - 1;
865895
}
@@ -872,14 +902,6 @@ class ARROW_EXPORT FixedSizeBinaryBuilder : public ArrayBuilder {
872902
int32_t byte_width_;
873903
BufferBuilder byte_builder_;
874904

875-
/// Temporary access to a value.
876-
///
877-
/// This pointer becomes invalid on the next modifying operation.
878-
uint8_t* GetMutableValue(int64_t i) {
879-
uint8_t* data_ptr = byte_builder_.mutable_data();
880-
return data_ptr + i * byte_width_;
881-
}
882-
883905
void CheckValueSize(int64_t size);
884906
};
885907

cpp/src/arrow/array/builder_primitive.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ class NumericBuilder
141141

142142
value_type GetValue(int64_t index) const { return data_builder_.data()[index]; }
143143

144+
value_type* GetMutableValue(int64_t index) {
145+
return &data_builder_.mutable_data()[index];
146+
}
147+
144148
void Reset() override {
145149
data_builder_.Reset();
146150
ArrayBuilder::Reset();
@@ -319,6 +323,28 @@ class NumericBuilder
319323
data_builder_.UnsafeAppend(value_type{}); // zero
320324
}
321325

326+
/// Advance builder without allocating nor writing any values
327+
///
328+
/// The internal pointer is advanced by `length` values and the same number
329+
/// of non-null entries are appended to the validity bitmap.
330+
/// This method assumes that the `length` values were populated directly,
331+
/// for example using `GetMutableValue`.
332+
void UnsafeAdvance(int64_t length) {
333+
data_builder_.UnsafeAdvance(length);
334+
UnsafeAppendToBitmap(length, true);
335+
}
336+
337+
/// Advance builder without allocating nor writing any values
338+
///
339+
/// The internal pointer is advanced by `length` values and the same number
340+
/// of validity bits are appended to the validity bitmap.
341+
/// This method assumes that the `length` values were populated directly,
342+
/// for example using `GetMutableValue`.
343+
void UnsafeAdvance(int64_t length, const uint8_t* validity, int64_t valid_bits_offset) {
344+
data_builder_.UnsafeAdvance(length);
345+
UnsafeAppendToBitmap(validity, valid_bits_offset, length);
346+
}
347+
322348
std::shared_ptr<DataType> type() const override { return type_; }
323349

324350
protected:

cpp/src/arrow/buffer_builder.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ class TypedBufferBuilder<
295295
return bytes_builder_.Advance(length * sizeof(T));
296296
}
297297

298+
void UnsafeAdvance(const int64_t length) {
299+
bytes_builder_.UnsafeAdvance(length * sizeof(T));
300+
}
301+
298302
Status Finish(std::shared_ptr<Buffer>* out, bool shrink_to_fit = true) {
299303
return bytes_builder_.Finish(out, shrink_to_fit);
300304
}

cpp/src/arrow/util/spaced_internal.h

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@
2323

2424
#include "arrow/util/bit_run_reader.h"
2525

26-
namespace arrow {
27-
namespace util {
28-
namespace internal {
26+
namespace arrow::util::internal {
2927

3028
/// \brief Compress the buffer to spaced, excluding the null entries.
3129
///
@@ -53,27 +51,29 @@ inline int SpacedCompress(const T* src, int num_values, const uint8_t* valid_bit
5351
return num_valid_values;
5452
}
5553

56-
/// \brief Relocate values in buffer into positions of non-null values as indicated by
57-
/// a validity bitmap.
54+
/// \brief Relocate values according to a validity bitmap, to the right
55+
///
56+
/// Non-null values should initially be densely packed at the left of the buffer.
57+
/// This method spreads the values out according to the given validity bitmap.
58+
/// Null entries are zero-initialized.
5859
///
5960
/// \param[in, out] buffer the in-place buffer
6061
/// \param[in] num_values total size of buffer including null slots
6162
/// \param[in] null_count number of null slots
6263
/// \param[in] valid_bits bitmap data indicating position of valid slots
6364
/// \param[in] valid_bits_offset offset into valid_bits
64-
/// \return The number of values expanded, including nulls.
6565
template <typename T>
66-
inline int SpacedExpand(T* buffer, int num_values, int null_count,
67-
const uint8_t* valid_bits, int64_t valid_bits_offset) {
66+
inline void SpacedExpandRightward(T* buffer, int num_values, int null_count,
67+
const uint8_t* valid_bits, int64_t valid_bits_offset) {
6868
// Point to end as we add the spacing from the back.
6969
int idx_decode = num_values - null_count;
7070

7171
// Depending on the number of nulls, some of the value slots in buffer may
7272
// be uninitialized, and this will cause valgrind warnings / potentially UB
73-
std::memset(static_cast<void*>(buffer + idx_decode), 0, null_count * sizeof(T));
73+
memset(static_cast<void*>(buffer + idx_decode), 0, null_count * sizeof(T));
7474
if (idx_decode == 0) {
7575
// All nulls, nothing more to do
76-
return num_values;
76+
return;
7777
}
7878

7979
arrow::internal::ReverseSetBitRunReader reader(valid_bits, valid_bits_offset,
@@ -85,14 +85,60 @@ inline int SpacedExpand(T* buffer, int num_values, int null_count,
8585
}
8686
idx_decode -= static_cast<int32_t>(run.length);
8787
assert(idx_decode >= 0);
88-
std::memmove(buffer + run.position, buffer + idx_decode, run.length * sizeof(T));
88+
if (idx_decode == run.position) {
89+
// We have come to the point where no more expansion is required: the remaining
90+
// values are already in their final position.
91+
return;
92+
}
93+
// Source and destination may overlap if run.length > 1
94+
memmove(buffer + run.position, buffer + idx_decode, run.length * sizeof(T));
8995
}
9096

9197
// Otherwise caller gave an incorrect null_count
9298
assert(idx_decode == 0);
93-
return num_values;
9499
}
95100

96-
} // namespace internal
97-
} // namespace util
98-
} // namespace arrow
101+
/// \brief Relocate values according to a validity bitmap, to the left
102+
///
103+
/// Non-null values should initially be densely packed at the right of the buffer.
104+
/// This method spreads the values out according to the given validity bitmap.
105+
/// Null entries are zero-initialized.
106+
///
107+
/// \param[in, out] buffer the in-place buffer
108+
/// \param[in] byte_width the byte width of values
109+
/// \param[in] length total length of buffer including null slots
110+
/// \param[in] null_count number of null slots
111+
/// \param[in] valid_bits bitmap data indicating position of valid slots
112+
/// \param[in] valid_bits_offset offset into valid_bits
113+
inline void SpacedExpandLeftward(uint8_t* buffer, int byte_width, int64_t length,
114+
int64_t null_count, const uint8_t* valid_bits,
115+
int64_t valid_bits_offset) {
116+
// Point to start of values.
117+
int64_t idx_decode = byte_width * null_count;
118+
119+
// Depending on the number of nulls, some of the value slots in buffer may
120+
// be uninitialized, and this will cause valgrind warnings / potentially UB
121+
memset(buffer, 0, idx_decode);
122+
123+
arrow::internal::SetBitRunReader reader(valid_bits, valid_bits_offset, length);
124+
while (true) {
125+
const auto run = reader.NextRun();
126+
if (run.length == 0) {
127+
break;
128+
}
129+
if (idx_decode == run.position * byte_width) {
130+
// We have come to the point where no more expansion is required: the remaining
131+
// values are already in their final position.
132+
return;
133+
}
134+
// Source and destination may overlap if run.length > 1
135+
memmove(buffer + run.position * byte_width, buffer + idx_decode,
136+
run.length * byte_width);
137+
idx_decode += run.length * byte_width;
138+
}
139+
140+
// Otherwise caller gave an incorrect null_count
141+
assert(idx_decode == length * byte_width);
142+
}
143+
144+
} // namespace arrow::util::internal

cpp/src/parquet/arrow/reader_writer_benchmark.cc

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -417,22 +417,34 @@ BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType)
417417
->Args({5, 10});
418418

419419
//
420-
// Benchmark reading a PLAIN-encoded primitive column
420+
// Benchmark reading a non-dict-encoded primitive column
421421
//
422422

423-
template <bool nullable, typename ParquetType>
424-
static void BM_ReadColumnPlain(::benchmark::State& state) {
423+
template <typename ParquetType>
424+
static void BenchmarkReadNonDictColumn(::benchmark::State& state, bool nullable,
425+
Encoding::type encoding) {
425426
using c_type = typename ArrowType<ParquetType>::c_type;
426427

427428
const std::vector<c_type> values(BENCHMARK_SIZE, static_cast<c_type>(42));
428429
std::shared_ptr<Table> table =
429430
TableFromVector<ParquetType>(values, /*nullable=*/nullable, state.range(0));
430431

431-
auto properties = WriterProperties::Builder().disable_dictionary()->build();
432+
auto properties =
433+
WriterProperties::Builder().disable_dictionary()->encoding(encoding)->build();
432434
BenchmarkReadTable(state, *table, properties, table->num_rows(),
433435
BytesForItems<ParquetType>(table->num_rows()));
434436
}
435437

438+
template <bool nullable, typename ParquetType>
439+
static void BM_ReadColumnPlain(::benchmark::State& state) {
440+
BenchmarkReadNonDictColumn<ParquetType>(state, nullable, Encoding::PLAIN);
441+
}
442+
443+
template <bool nullable, typename ParquetType>
444+
static void BM_ReadColumnByteStreamSplit(::benchmark::State& state) {
445+
BenchmarkReadNonDictColumn<ParquetType>(state, nullable, Encoding::BYTE_STREAM_SPLIT);
446+
}
447+
436448
BENCHMARK_TEMPLATE2(BM_ReadColumnPlain, false, Int32Type)
437449
->ArgNames({"null_probability"})
438450
->Args({kAlternatingOrNa});
@@ -455,12 +467,24 @@ BENCHMARK_TEMPLATE2(BM_ReadColumnPlain, true, Float16LogicalType)
455467
->Args({99})
456468
->Args({100});
457469

470+
BENCHMARK_TEMPLATE2(BM_ReadColumnByteStreamSplit, false, Float16LogicalType)
471+
->ArgNames({"null_probability"})
472+
->Args({kAlternatingOrNa});
473+
BENCHMARK_TEMPLATE2(BM_ReadColumnByteStreamSplit, true, Float16LogicalType)
474+
->ArgNames({"null_probability"})
475+
->Args({0})
476+
->Args({1})
477+
->Args({50})
478+
->Args({99})
479+
->Args({100});
480+
458481
//
459482
// Benchmark reading binary column
460483
//
461484

462485
static void BenchmarkReadBinaryColumn(::benchmark::State& state,
463-
const std::shared_ptr<::arrow::DataType>& type) {
486+
const std::shared_ptr<::arrow::DataType>& type,
487+
Encoding::type encoding) {
464488
std::shared_ptr<Table> table =
465489
RandomStringTable(type, BENCHMARK_SIZE, state.range(1), state.range(0));
466490

@@ -470,7 +494,9 @@ static void BenchmarkReadBinaryColumn(::benchmark::State& state,
470494
for (size_t i = 1; i < column.buffers.size(); ++i) {
471495
total_bytes += column.buffers[i]->size();
472496
}
473-
BenchmarkReadTable(state, *table, table->num_rows(), total_bytes);
497+
498+
auto properties = WriterProperties::Builder().encoding(encoding)->build();
499+
BenchmarkReadTable(state, *table, properties, table->num_rows(), total_bytes);
474500
}
475501

476502
static void SetReadBinaryColumnArgs(benchmark::internal::Benchmark* b) {
@@ -487,17 +513,41 @@ static void SetReadBinaryColumnArgs(benchmark::internal::Benchmark* b) {
487513
->Args({99, kInfiniteUniqueValues});
488514
}
489515

516+
static void SetReadBinaryColumnArgsWithoutDictEncoding(
517+
benchmark::internal::Benchmark* b) {
518+
b->ArgNames({"null_probability", "unique_values"})
519+
// Dict-encoding is already tested in the PLAIN benchmarks, so only exercise
520+
// non-dict-encoding using high cardinality.
521+
->Args({0, kInfiniteUniqueValues})
522+
->Args({1, kInfiniteUniqueValues})
523+
->Args({50, kInfiniteUniqueValues})
524+
->Args({99, kInfiniteUniqueValues});
525+
}
526+
490527
static void BM_ReadBinaryColumn(::benchmark::State& state) {
491-
BenchmarkReadBinaryColumn(state, ::arrow::utf8());
528+
BenchmarkReadBinaryColumn(state, ::arrow::utf8(), Encoding::PLAIN);
492529
}
493530

494531
static void BM_ReadBinaryViewColumn(::benchmark::State& state) {
495-
BenchmarkReadBinaryColumn(state, ::arrow::large_utf8());
532+
BenchmarkReadBinaryColumn(state, ::arrow::large_utf8(), Encoding::PLAIN);
533+
}
534+
535+
static void BM_ReadBinaryColumnDeltaByteArray(::benchmark::State& state) {
536+
BenchmarkReadBinaryColumn(state, ::arrow::utf8(), Encoding::DELTA_BYTE_ARRAY);
537+
}
538+
539+
static void BM_ReadBinaryViewColumnDeltaByteArray(::benchmark::State& state) {
540+
BenchmarkReadBinaryColumn(state, ::arrow::large_utf8(), Encoding::DELTA_BYTE_ARRAY);
496541
}
497542

498543
BENCHMARK(BM_ReadBinaryColumn)->Apply(SetReadBinaryColumnArgs);
499544
BENCHMARK(BM_ReadBinaryViewColumn)->Apply(SetReadBinaryColumnArgs);
500545

546+
BENCHMARK(BM_ReadBinaryColumnDeltaByteArray)
547+
->Apply(SetReadBinaryColumnArgsWithoutDictEncoding);
548+
BENCHMARK(BM_ReadBinaryViewColumnDeltaByteArray)
549+
->Apply(SetReadBinaryColumnArgsWithoutDictEncoding);
550+
501551
//
502552
// Benchmark reading a nested column
503553
//

0 commit comments

Comments
 (0)