GH-55399: [C++][Parquet] Minor optimize level counting with CounfIf16 by mapleFU · Pull Request #55408 · apache/arrow

mapleFU · 2026-11-05T05:19:57Z

Rationale for this change

std::count with int16_t boolean is hard to optimize. So, adding a CountIf16 here.

x86: https://godbolt.org/z/Gqc7fxe55
ARM: https://godbolt.org/z/sGhG8crYa

Before x86:

vmovq      (%rdi,%rax,2), %xmm6      # Load 8 bytes = 4  levels
vpcmpeqw   %xmm1, %xmm6, %xmm6
vpmovzxwq  %xmm6, %ymm6              # 8×16 bits→ 4×64 bits，only 4 lanes
vpand      %ymm2, %ymm6, %ymm6       # & 1
vpaddq     %ymm6, %ymm0, %ymm0       # 64 bits adds

After x86:

vpcmpeqw   (%rdi,%r8,2), %ymm1, %ymm5   # compare 16 levels
vpsubw     %ymm5, %ymm0, %ymm0          # add 16 lanes

Before ARM:

ldp q18,q19,[x8,#-16]      ; 2 cmeq 16 bits mask
cmeq v18.8h, v18.8h, v16.8h
cmeq v19.8h, v19.8h, v16.8h
ushll2 v20.4s, v18.8h, #0  ; 16→32 bits
ushll  v18.4s, v18.4h, #0
...
and    v22.16b, v22.16b, v17.16b   ;
add    v4.2d, v4.2d, v20.2d        ;
subs x14,x14,#16 ; b.ne

After arm:

ldp q3,q4,[x9,#-16]
cmeq v3.8h, v3.8h, v2.8h
cmeq v4.8h, v4.8h, v2.8h
sub  v0.8h, v0.8h, v3.8h
sub  v1.8h, v1.8h, v4.8h
subs/add/b.ne

Some notes:

When levels is not too many ( like < 32 on my M4 Pro MacOS ), the instruction cannot be vectorized, and introduce more checkings, and would be slower, so I introduce a < 32 here
It optimize both read/write, but this is not critical path, so optimization would not be too much in e2e benchmarking

What changes are included in this PR?

Adding a CounfIf16 to optimize level counting.

Are these changes tested?

Yes and covered by existing.

Are there any user-facing changes?

No

Was AI used for this PR?

PR code and description written by:

Human
AI

Reviewed before submission by:

Human

GitHub Issue: [C++][Parquet] Minor Optimization: Optimize some counting about int16_t levels in parquet #55399

…ounting ### Rationale for this change Counting how many of a run of int16_t levels match a predicate (e.g. how many definition levels equal the max definition level, or how many repetition levels are 0) is a hot loop in the Parquet reader and writer. It is currently open-coded as a scalar loop or a std::count() call. `std::count` accumulates into a 64-bit integer, which forces the compiler to widen every 16-bit compare result before accumulating and prevents efficient vectorization. Accumulating into a `uint16_t` instead lets the compiler keep the whole reduction in 16-bit lanes (e.g. `cmeq.8h` + `sub.8h` + `addv.8h` on Neon, 32 levels per iteration), and the chunked loop keeps that accumulator from overflowing: at most `0xFFFF` levels are counted before the accumulator is folded into the 64-bit total. The chunk bookkeeping (clamping the chunk length, folding the partial count, advancing the pointer) is a fixed cost per call, and for inputs shorter than one vector iteration it dominates: counting 7 levels that way is measurably slower than `std::count`, even though both end up running the same scalar loop. Such short inputs are therefore counted directly, without going through the chunk loop. On an Apple M-series machine, counting 2048 int16 levels where every tenth is missing drops from ~232 ns to ~32 ns (about 7x), while short inputs are at least as fast as `std::count`. ### What changes are included in this PR? - Add `parquet::internal::CountIf16` next to the other level scanning primitives in `parquet/level_comparison.h`. It is a header-only inline template, since the level conversion translation unit is compiled twice (standard and BMI2). - Add unit tests covering empty input, a mixed predicate, short inputs around the vectorization threshold, and the `0xFFFF` chunk boundaries. - Add `BM_CountPresentLevels{StdCount,CountIf16}` to the level conversion benchmark to track the improvement. ### Are these changes tested? New `CountIf16.*` tests in `level_conversion_test.cc`, run as part of `parquet-reader-test`. ### Are there any user-facing changes? No.

github-actions · 2026-11-05T05:22:59Z

⚠️ GitHub issue #55399 has been automatically assigned in GitHub to PR creator.

Copilot

🟢 Approval recommended

No active review issues are reported for the shared Parquet level-counting helper and its integrations.

No open issues 🎉

What changed in this PR

This PR addresses GH-55399 by introducing a vectorization-friendly helper for counting Parquet definition and repetition levels, then applying it across reading, writing, and size statistics.

Changes:

Add CountIf16 with 16-bit accumulators, overflow-safe chunking, and a short-input path.
Replace existing counting loops and std::count calls with the shared helper.
Add tests for predicates, short inputs, and chunk boundaries.

File	Description
`cpp/src/parquet/size_statistics.cc`	Use the helper for binary-level histogram counts.
`cpp/src/parquet/level_conversion_test.cc`	Test counting correctness and chunk boundaries.
`cpp/src/parquet/level_comparison.h`	Implement `CountIf16` and its chunked fallback.
`cpp/src/parquet/file_reader.cc`	Use the helper to count rows during file scans.
`cpp/src/parquet/column_writer.cc`	Replace value and row counting loops with the helper.
`cpp/src/parquet/column_reader.cc`	Apply the helper to value counting, record delimiting, and skipping.

_{Review effort: Balanced}

HuaHuaY · 2026-11-05T08:39:21Z

It looks like an interesting change. It might be worth submitting to LLVM to optimize the implementation of std::count in the standard library.

mapleFU · 2026-11-05T08:59:06Z

There're existing discussion about this, see: llvm/llvm-project#204897

But anyway if you want to benefit from this, you need at least clang > 23.1, and I don't know when could LLVM implement this :-(

HuaHuaY · 2026-11-05T08:59:11Z

+  // unrolled ladder of 8/16/24-wide compares instead of scalar code.
+  constexpr int64_t kSmallLevels = 32;
+  constexpr int64_t kMaxCount = std::numeric_limits<uint16_t>::max();
+  if (num_levels < kSmallLevels) {


Does it make sense to keep this if branch, rather than letting LLVM automatically generate optimized code for short loops?

I don't know, it would making the very short loops (9-31 elements?) a bit slower ( like 1.5ns -> 3ns ) on my Mac M4 Pro. If this doesn't matter just counting by chunk is ok.

HuaHuaY · 2026-11-05T09:11:27Z

+    // 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,


My LLM pointed out a performance regression here, and I think it’s right. There’s a logical constraint—each level can only be 1 or 0, so a summation could replace std::count. However, the compiler can’t infer this, and the new code ends up performing a check (level == 1 ? 1 : 0), which actually makes it slower.

aha, nice catch. I would try whether an ASSUME helps

I think similar optimizations may be applied elsewhere in this PR. Add a branch for descr_->max_repetition_level() == 1 and calcuate the result via num_levels - std::accumulate instead of std::count.

I've tried it, but the optimization wouldn't be many. And I just found only that user case. Seems user can currently just hand-written accumulate here.🤔

I’ve found that the ASSUME macro can easily resolve this issue https://godbolt.org/z/Mf1EzdPP1. However, the better approach here might be to leave the original code unmodified.

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...

Copilot

🟢 Approval recommended

No active review issues are reported for the Parquet level-counting optimization.

No open issues 🎉

_{Review effort: Balanced}

…ader and writer ### Rationale for this change Follow-up to the `CountIf16` helper: use it for the level counting loops in the Parquet column reader/writer and in the file scanner, replacing open-coded scalar loops and `std::count` calls. Converted sites: - `TypedColumnReaderImpl::ReadLevelsInCurrentPage`: count non-null values. - `TypedRecordReader::SkipRecordsInBufferNonRepeated`: count values to skip. - `TypedRecordReader::DelimitRecords`: count records (rep level == 0) and values seen (def level == max). - `TypedRecordReader::ReadDenseForOptionalInBuffer`: count values to read. - `TypedColumnWriterImpl::WriteLevels`: count values to write (both overloads) and records written (rep level == 0). - `TypedColumnWriterImpl::MaybeCalculateValidityBits`: count values to write and spaced values to write. - `ScanFileContents`: count rows (rep level == 0). Sites are left as is where counting is not the bottleneck: the per-element bitmap loops in `DefRepLevelsToListInfo` also build validity bitmaps and write list offsets for every level, `page_index.cc` counts pages rather than levels, and the non-nullable validation loop in `WriteArrow` needs to `break` early on the first mismatch. `UpdateLevelHistogram`'s `max_level == 1` fast path is also left as is: its levels are known to be 0 or 1, so it sums them with `std::accumulate` in the `int16_t` space, which is a single 16-bit add per vector, while counting the 1s costs a compare and a subtract. ### What changes are included in this PR? Use `CountIf16` for the level counting call sites above. ### Are these changes tested? Covered by the existing Parquet reader, writer and Arrow reader/writer tests. ### Are there any user-facing changes? No.

Copilot

🟢 Approval recommended

No active review issues block the Parquet level-counting optimization.

No open issues 🎉

_{Review effort: Balanced}

github-actions Bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Nov 5, 2026

mapleFU marked this pull request as ready for review November 5, 2026 07:10

mapleFU requested review from HuaHuaY, pitrou and wgtmac as code owners November 5, 2026 07:10

Copilot AI balanced review requested due to automatic review settings November 5, 2026 07:10

Copilot started reviewing on behalf of mapleFU November 5, 2026 07:11

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

HuaHuaY reviewed Nov 5, 2026

View reviewed changes

github-actions Bot added awaiting review Awaiting review awaiting committer review Awaiting committer review and removed awaiting review Awaiting review awaiting committer review Awaiting committer review labels Nov 5, 2026

Copilot AI balanced review requested due to automatic review settings November 5, 2026 12:22

Copilot started reviewing on behalf of mapleFU November 5, 2026 12:22

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

mapleFU force-pushed the parquet/gh-55399-level-count-if16 branch from e1291b9 to 7ce5324 Compare November 5, 2026 12:35

Copilot AI balanced review requested due to automatic review settings November 5, 2026 12:35

Copilot started reviewing on behalf of mapleFU November 5, 2026 12:36

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Conversation

mapleFU commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Was AI used for this PR?

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

HuaHuaY commented Nov 5, 2026

Uh oh!

mapleFU commented Nov 5, 2026

Uh oh!

HuaHuaY Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

mapleFU Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

HuaHuaY Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

mapleFU Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

HuaHuaY Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

mapleFU Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

HuaHuaY Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mapleFU Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mapleFU commented Nov 5, 2026 •

edited

Loading

HuaHuaY Nov 5, 2026 •

edited

Loading