mapleFU commented on code in PR #55408:
URL: https://github.com/apache/arrow/pull/55408#discussion_r4482430313
##########
cpp/src/parquet/size_statistics.cc:
##########
@@ -160,17 +160,11 @@ void UpdateLevelHistogram(std::span<const int16_t>
levels, std::span<int64_t> hi
if (max_level == 1) {
// Specialize the common case for non-repeated non-nested columns.
- // Summing the levels gives us the number of 1s, and the number of 0s
follows.
- // We do repeated sums in the int16_t space, which the compiler is likely
- // to vectorize efficiently.
- constexpr int64_t kChunkSize = 1 << 14; // to avoid int16_t overflows
- int64_t hist1 = 0;
- auto it = levels.begin();
- while (it != levels.end()) {
- const auto chunk_size = std::min<int64_t>(levels.end() - it, kChunkSize);
- hist1 += std::accumulate(it, it + chunk_size, int16_t{0});
- it += chunk_size;
- }
+ // Counting the 1s directly gives us the number of 1s, and the number of 0s
+ // follows. CountIf16 accumulates in the int16_t space in chunks, which the
+ // compiler vectorizes efficiently and which cannot overflow.
+ const int64_t hist1 = internal::CountIf16(levels.data(), num_levels,
Review Comment:
Yes, I think the problem is that there is only one case here, and for gcc <
13, it doesn't have assume 🤔 So maybe leaving it here is ok...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
