You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tennisleng (via GitHub)" <gi...@apache.org> on 2025/12/15 23:51:37 UTC
[PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
tennisleng opened a new pull request, #48549:
URL: https://github.com/apache/arrow/pull/48549
### Rationale for this change
This PR implements the optimization to fuse definition level decoding with counting in the Parquet column reader, addressing the TODO in `cpp/src/parquet/column_reader.cc`.
### What changes are included in this PR?
1. Added `GetBatchWithCount` to `RleBitPackedDecoder`, `RleRunDecoder`, and `BitPackedRunDecoder` in `cpp/src/arrow/util/rle_encoding_internal.h`.
2. Added `DecodeAndCount` to `LevelDecoder` in `cpp/src/parquet/column_reader.h` and `cpp/src/parquet/column_reader.cc`.
3. Updated `TypedColumnReaderImpl::ReadLevels` in `cpp/src/parquet/column_reader.cc` to use `DecodeAndCount`.
### Are these changes tested?
Yes.
1. Added a new unit test `Rle.GetBatchWithCount` in `cpp/src/arrow/util/rle_encoding_test.cc`.
2. Verified with `arrow-bit-utility-test`.
### Are there any user-facing changes?
No, this is an internal performance optimization.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48549:
URL: https://github.com/apache/arrow/pull/48549#issuecomment-3658082314
:warning: GitHub issue #45847 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "AntoinePrv (via GitHub)" <gi...@apache.org>.
AntoinePrv commented on code in PR #48549:
URL: https://github.com/apache/arrow/pull/48549#discussion_r2630255991
##########
cpp/src/arrow/util/rle_encoding_internal.h:
##########
@@ -308,6 +308,23 @@ class RleRunDecoder {
return to_read;
}
+ /// Get a batch of values and count how many equal match_value
+ [[nodiscard]] rle_size_t GetBatchWithCount(value_type* out, rle_size_t batch_size,
+ rle_size_t value_bit_width,
+ value_type match_value, int64_t* out_count) {
+ if (ARROW_PREDICT_FALSE(remaining_count_ == 0)) {
+ return 0;
+ }
+
+ const auto to_read = std::min(remaining_count_, batch_size);
+ std::fill(out, out + to_read, value_);
+ if (value_ == match_value) {
+ *out_count += to_read;
+ }
+ remaining_count_ -= to_read;
+ return to_read;
+ }
+
Review Comment:
Could this call `RleRunDecoder::GetBatch` to avoid duplicating the logic?
##########
cpp/src/arrow/util/rle_encoding_internal.h:
##########
@@ -377,6 +394,19 @@ class BitPackedRunDecoder {
return steps;
}
+ /// Get a batch of values and count how many equal match_value
+ /// Note: For bit-packed runs, we use std::count after GetBatch since it's
+ /// highly optimized by the compiler. The fused approach is only beneficial
+ /// for RLE runs where counting is O(1).
+ [[nodiscard]] rle_size_t GetBatchWithCount(value_type* out, rle_size_t batch_size,
+ rle_size_t value_bit_width,
+ value_type match_value, int64_t* out_count) {
+ const auto steps = GetBatch(out, batch_size, value_bit_width);
+ // std::count is highly optimized (SIMD) by modern compilers
+ *out_count += std::count(out, out + steps, match_value);
Review Comment:
I have been working on the `unpack` function used in `GetBatch` and my intuition is also that this function could not easily be extended to count at the same time as it extract (not impossible but heavy changes).
Still this could provide better data locality when doing run by run.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #48549:
URL: https://github.com/apache/arrow/pull/48549#issuecomment-3665534299
Two things:
1. You should show benchmark numbers to make sure that this is actually an optimization (and that it brings benefits large enough)
2. Some work on level decoding is also being done in https://github.com/apache/arrow/pull/47994 , hopefully the two PRs won't conflict
cc @AntoinePrv FYI
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "tennisleng (via GitHub)" <gi...@apache.org>.
tennisleng commented on PR #48549:
URL: https://github.com/apache/arrow/pull/48549#issuecomment-3662291244
sorry for the confusion. i stumbled onto this optimization while investigating #45847, but you're right that they're not really related. this is more of a standalone parquet reader improvement. i can create a separate issue for it and update the reference if you'd like.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #48549:
URL: https://github.com/apache/arrow/pull/48549#issuecomment-3659124988
Thanks for creating the PR! I'm just confused about its relationship with the linked issue.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #48549:
URL: https://github.com/apache/arrow/pull/48549#discussion_r2630368777
##########
cpp/src/arrow/util/rle_encoding_internal.h:
##########
@@ -377,6 +394,19 @@ class BitPackedRunDecoder {
return steps;
}
+ /// Get a batch of values and count how many equal match_value
+ /// Note: For bit-packed runs, we use std::count after GetBatch since it's
+ /// highly optimized by the compiler. The fused approach is only beneficial
+ /// for RLE runs where counting is O(1).
+ [[nodiscard]] rle_size_t GetBatchWithCount(value_type* out, rle_size_t batch_size,
+ rle_size_t value_bit_width,
+ value_type match_value, int64_t* out_count) {
+ const auto steps = GetBatch(out, batch_size, value_bit_width);
+ // std::count is highly optimized (SIMD) by modern compilers
+ *out_count += std::count(out, out + steps, match_value);
Review Comment:
The typical batch size for levels is probably small, so it would fit at least in L2 cache and perhaps L1. Not sure it's worth trying to do it while decoding.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-45847: [C++] Optimize Parquet column reader by fusing decoding and counting [arrow]
Posted by "AntoinePrv (via GitHub)" <gi...@apache.org>.
AntoinePrv commented on PR #48549:
URL: https://github.com/apache/arrow/pull/48549#issuecomment-3669355317
> 2\. Some work on level decoding is also being done in
They do not
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org