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

Also, the original loop is already chunked correctly.

I'll revert this, since this will make this performance worse :-(

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}

pitrou · 2026-11-09T10:07:04Z

Does this even make a difference in a benchmark? There are so many expensive things happening when reading or writing Parquet (such as compression or encoding) that I doubt the cost of std::count is important...

pitrou · 2026-11-09T10:10:05Z

@ursabot please benchmark lang=C++

rok · 2026-11-09T10:10:12Z

Benchmark runs are scheduled for commit 7ce5324. Watch https://buildkite.com/apache-arrow and https://conbench.arrow-dev.org for updates. A comment will be posted here when the runs are complete.

mapleFU · 2026-11-09T10:13:03Z

Member
Does this even make a difference in a benchmark? There are so many expensive things happening when reading or writing Parquet (such as compression or encoding) that I doubt the cost of std::count is important...

Generally speaking, no. Some optimization might benifit from same optimization in #55308 , write side previously counting the numbers toooo slow. It has only a few optimization during read =_=

pitrou · 2026-11-09T14:34:31Z

Hmm, there seem to be some small but real speedups on the benchmarks.

pitrou · 2026-11-09T14:36:09Z

+/// It is kept out of line so that the common path (a single Parquet page chunk,
+/// always short enough for one accumulator) is a plain loop without any chunk
+/// bookkeeping in front of it.


Was it measured to make a difference?
Was this AI-generated?

Was it measured to make a difference?

Generally speaking yes. The asm and benchmark shows this could benefit the performance.

Was this AI-generated?

Yes, but all changes are verified and reviewed myself

pitrou · 2026-11-09T14:37:04Z

+  // Short inputs are counted first, before the wide loop and before the chunk
+  // clamp, because a handful of levels cannot amortize either. Running the
+  // small case first is also how folly::simd::detail::simdForEach orders its
+  // single steps before the unrolled ones, so that short inputs never pay for
+  // the unrolled set up.
+  //
+  // 32 is the widest per-iteration body this reduction gets: a packed int16_t
+  // register holds 8 levels with SSE2/NEON and 16 with AVX2, but clang runs up
+  // to 32 levels per iteration on NEON (four registers), so shorter inputs
+  // never reach a full iteration on the narrow targets. The bound also has to
+  // be a compile-time power of two, which is what turns the loop into an
+  // unrolled ladder of 8/16/24-wide compares instead of scalar code.


This lengthy comment is AI-generated, right?

Yeah, I'll rewrite. it myself

pitrou · 2026-11-09T14:37:24Z

+  // 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) {


What's the point of this if? It's exactly the same code as the other loop below.

I found this nice for small benchmark, actuall when if (n < 32), directly loop is slower than std::count.
n < 32 will tell the compiler to unroll the loop for it. when n <= 8 it's slower, when n in (8, 32), this making the counting faster.

Without n < 32 ( or this small check ), it just fall into loop.

Detail see: https://godbolt.org/z/nj1vKvqPY

Performance on my MacOS:

n u16_guard u16 i64_guard std_count i64_loop

1 2.38 1.53 1.93 1.43 1.14

4 3.16 2.29 3.15 1.89 2.06

7 4.12 3.82 4.00 3.45 3.83

8 1.76 2.35 2.00 2.29 2.65

13 3.73 3.14 4.13 3.78 3.18

16 2.00 3.43 2.80 3.52 3.81

24 1.99 4.02 4.21 4.51 4.41

28 3.81 4.28 5.00 5.12 5.02

31 4.69 5.87 5.87 6.17 6.15

32 2.05 2.35 6.76 6.63 6.56

256 5.58 5.72 37.60 38.70 37.81

2048 38.70 39.25 278.86 277.19 283.70

I can remove this if we think it's a benchmark hack

Detail see: https://godbolt.org/z/nj1vKvqPY

Micro-benchmarks like this do not tell us about performance when called from Parquet C++ reader code. Inlining may be different, code placement may be different, etc.

Yeah, the main difference is that it helps unroll n<32's vectorization, and tells the compiler the loop is for element >32 && <15536. But I can remove this, any way CountIf16 mainly benefit from Chunked counting.

pitrou · 2026-11-09T14:40:06Z


 MinMax PARQUET_EXPORT FindMinMax(const int16_t* levels, int64_t num_levels);

+/// Counts the levels that satisfy `pred`, in chunks small enough that a


Can you move these functions to a private/internal header?

Emmm it's already parquet::internal in a internal header?

Hmm, the header is not internal, but that can be a separate issue.

pitrou · 2026-11-09T14:40:53Z

+///   `bool(int16_t)`.
+/// \return The number of levels for which `pred` returns true.
+template <typename Predicate>
+inline int64_t CountIf16(const int16_t* levels, int64_t num_levels, Predicate pred) {


Should be Predicate&& pred?

pitrou · 2026-11-09T14:41:20Z

+    return count;
+  }
+  if (num_levels > kMaxCount) {
+    return CountIf16Chunked(levels, num_levels, pred);


Should use perfect forwarding?

Suggested change

return CountIf16Chunked(levels, num_levels, pred);

return CountIf16Chunked(levels, num_levels, std::forward<Predicate>(pred));

pitrou · 2026-11-09T14:46:22Z

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


Also, the original loop is already chunked correctly.

mapleFU · 2026-11-09T15:48:51Z

It's a bit late in my tz, I'll resolve comments tomorrow night

github-actions Bot added Component: Parquet Component: C++ 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 and removed awaiting 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

pitrou reviewed Nov 9, 2026

View reviewed changes

raulcd removed the awaiting committer review label Nov 10, 2026

n	u16_guard	u16	i64_guard	std_count	i64_loop
1	2.38	1.53	1.93	1.43	1.14
4	3.16	2.29	3.15	1.89	2.06
7	4.12	3.82	4.00	3.45	3.83
8	1.76	2.35	2.00	2.29	2.65
13	3.73	3.14	4.13	3.78	3.18
16	2.00	3.43	2.80	3.52	3.81
24	1.99	4.02	4.21	4.51	4.41
28	3.81	4.28	5.00	5.12	5.02
31	4.69	5.87	5.87	6.17	6.15
32	2.05	2.35	6.76	6.63	6.56
256	5.58	5.72	37.60	38.70	37.81
2048	38.70	39.25	278.86	277.19	283.70


		MinMax PARQUET_EXPORT FindMinMax(const int16_t* levels, int64_t num_levels);

		/// Counts the levels that satisfy `pred`, in chunks small enough that a

	return CountIf16Chunked(levels, num_levels, pred);
	return CountIf16Chunked(levels, num_levels, std::forward<Predicate>(pred));

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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

Uh oh!

Uh oh!

pitrou commented Nov 9, 2026

Uh oh!

rok commented Nov 9, 2026

Uh oh!

mapleFU commented Nov 9, 2026

Uh oh!

pitrou commented Nov 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mapleFU commented Nov 5, 2026 •

edited

Loading

HuaHuaY Nov 5, 2026 •

edited

Loading

pitrou commented Nov 9, 2026 •

edited

Loading