diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index c4d3fe5a8a5a..2fa25554273b 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1601,6 +1601,33 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { values_remaining_current_mini_block_ = values_per_mini_block_; } + // The miniblocks of a block are packed back to back with no padding between them, + // so a run of miniblocks that share a bit width is bit-identical to a single + // longer run at that width, and can be unpacked in one call. Returns how many + // whole miniblocks following the current one may be folded into it, given how many + // more values the caller has room for. + // + // A miniblock joins the run only when its bit width equals delta_bit_width_, which + // InitMiniBlock has already validated. Coalescing therefore never depends on a + // width that has not been checked, including the non-conformant widths InitBlock + // tolerates for extraneous miniblocks. + uint32_t CoalescibleMiniBlocks(uint32_t values_available) const { + // Folding in a whole miniblock first requires room for the current one in full. + if (values_available < values_remaining_current_mini_block_) { + return 0; + } + const uint8_t* bit_widths = delta_bit_widths_->data(); + uint32_t values_needed = values_remaining_current_mini_block_; + uint32_t count = 0; + while (mini_block_idx_ + count + 1 < mini_blocks_per_block_ && + bit_widths[mini_block_idx_ + count + 1] == delta_bit_width_ && + values_available - values_needed >= values_per_mini_block_) { + values_needed += values_per_mini_block_; + ++count; + } + return count; + } + int GetInternal(T* buffer, int max_values) { max_values = static_cast(std::min(max_values, total_values_remaining_)); if (max_values == 0) { @@ -1642,8 +1669,22 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { } } - int values_decode = std::min(values_remaining_current_mini_block_, - static_cast(max_values - i)); + const uint32_t values_available = static_cast(max_values - i); + const uint32_t values_this_mini_block = + std::min(values_remaining_current_mini_block_, values_available); + // The default geometry is 32 values per miniblock, and a call that small is + // mostly per-call setup for the unpacker; folding a run of four into one call + // asks it for 128 values instead. A zero bit width decodes without asking the + // unpacker at all, so there is no call to fold and nothing to gain. + const uint32_t mini_blocks_coalesced = + delta_bit_width_ == 0 ? 0 : CoalescibleMiniBlocks(values_available); + // A miniblock is only folded in when there is room for the current one in + // full, so a non-empty run means this call drains the current miniblock along + // with every miniblock folded into it. The accounting below relies on that. + DCHECK(mini_blocks_coalesced == 0 || + values_this_mini_block == values_remaining_current_mini_block_); + const int values_decode = static_cast( + values_this_mini_block + mini_blocks_coalesced * values_per_mini_block_); if (delta_bit_width_ == 0) { // Fast path that avoids a back-to-back dependency between two consecutive // computations: we know all deltas decode to zero. We actually don't @@ -1666,7 +1707,11 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { last_value_ = buffer[i + j]; } } - values_remaining_current_mini_block_ -= values_decode; + // A coalesced call drained the miniblocks it folded in, so advance the block's + // cursor past them: the last miniblock of the run becomes the current one, + // with nothing left in it. + mini_block_idx_ += mini_blocks_coalesced; + values_remaining_current_mini_block_ -= values_this_mini_block; i += values_decode; } total_values_remaining_ -= max_values; diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 831829e4a210..39a5b6ecac98 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -1752,7 +1752,10 @@ class TestDeltaBitPackEncoding : public TestEncodingBase { using c_type = typename Type::c_type; static constexpr int TYPE = Type::type_num; static constexpr size_t kNumRoundTrips = 3; - const std::vector kReadBatchSizes = {1, 11}; + // 1 and 11 stop partway through a miniblock; 100 spans several of them but still + // ends inside one, so a decoder that unpacks whole miniblocks at a time has to + // both use and give up that shortcut within a single read. + const std::vector kReadBatchSizes = {1, 11, 100}; void InitBoundData(int nvalues, int repeats, c_type half_range) { num_values_ = nvalues * repeats; @@ -2034,6 +2037,83 @@ TYPED_TEST(TestDeltaBitPackEncoding, ZeroDeltaBitWidth) { this->CheckRoundtripWithValues(int_values); } +TYPED_TEST(TestDeltaBitPackEncoding, MiniblockBitWidthRuns) { + // The miniblocks of a block are packed back to back with no padding between + // them, so a run of miniblocks sharing a bit width is bit-identical to one + // longer run at that width and a decoder may unpack the whole run in a single + // call. Cover the patterns that decide where such a run starts and stops: a + // block whose miniblocks all share a width, one where none of them do, runs + // that end partway through a block or at its boundary, and zero-width + // miniblocks, which the closed form decodes and which must not join a run. + using T = typename TypeParam::c_type; + + // Same values as in DeltaBitPackEncoder + constexpr int kValuesPerBlock = + std::is_same_v ? 128 : 256; + constexpr int kMiniBlocksPerBlock = 4; + constexpr int kValuesPerMiniBlock = kValuesPerBlock / kMiniBlocksPerBlock; + + // Produce values whose deltas give miniblock i the bit width widths[i]. Each + // miniblock alternates a delta of `frame` with a delta of `frame + 2^(w-1)`, + // and 2^(w-1) is the smallest value needing w bits, so the encoder stores width + // w for that miniblock. Every delta in the block is at least `frame`, which + // therefore becomes the frame the encoder stores; passing a negative one + // exercises a frame the decoder has to sign-extend. + auto make_values = [](const std::vector& widths, T frame, int trailing_values) { + std::vector values; + values.reserve(widths.size() * kValuesPerMiniBlock + trailing_values + 1); + // The first value travels in the header and contributes no delta. + T current = 0; + values.push_back(current); + for (const int width : widths) { + const T spread = width == 0 ? T{0} : static_cast(T{1} << (width - 1)); + for (int i = 0; i < kValuesPerMiniBlock; ++i) { + current = static_cast(current + frame + (i % 2 == 0 ? T{0} : spread)); + values.push_back(current); + } + } + // A tail shorter than a miniblock leaves the last block partly filled, so a + // run has to stop at the end of the encoded values rather than at a change of + // width. + for (int i = 0; i < trailing_values; ++i) { + current = static_cast(current + frame); + values.push_back(current); + } + return values; + }; + + struct Case { + const char* name; + std::vector widths; + int trailing_values; + }; + const std::vector cases = { + // One run covering every miniblock of the block. + {"uniform widths", {4, 4, 4, 4}, 0}, + // No two neighbours share a width, so no run forms. + {"no repeated width", {1, 8, 3, 16}, 0}, + // Runs that end partway through the block. + {"two runs of two", {1, 1, 8, 8}, 0}, + {"run then a change", {4, 4, 4, 16}, 0}, + // Zero-width miniblocks beside a run. + {"zero widths first", {0, 0, 3, 3}, 0}, + {"zero widths last", {3, 3, 0, 0}, 0}, + {"zero width inside a run", {3, 0, 3, 3}, 0}, + // Widths match either side of a block boundary, where a run must still stop + // because the bit widths belong to their own block. + {"across a block boundary", {4, 4, 4, 4, 4, 4, 4, 4}, 0}, + // A final block that ends in the middle of a miniblock. + {"partial last block", {4, 4, 4, 4}, 5}, + }; + + for (const auto& c : cases) { + for (const T frame : {T{0}, static_cast(-5)}) { + ARROW_SCOPED_TRACE("case = ", c.name, ", frame = ", static_cast(frame)); + this->CheckRoundtripWithValues(make_values(c.widths, frame, c.trailing_values)); + } + } +} + // ---------------------------------------------------------------------- // Rle for Boolean encode/decode tests.