GH-50784: [C++] Align write in TransferBitmap

[AntoinePrv]

Rationale for this change

Faster without much more complexity.
Locally (Macbook Pro M3), I'm getting 100% speedup on CopyBitmapWithOffsetBoth.

What changes are included in this PR?

Align the writer in transfer bitmap for more efficient writes in bitmap unary and binary operation.
Simplify the use of bit functions as templates.

Are these changes tested?

Yes with existing tests.

Are there any user-facing changes?

No.

• GitHub Issue: [C++] Speed up CopyBitmap/InvertBitmap #50784

[github-actions]

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

[Copilot]

Pull request overview

This PR optimizes TransferBitmap (used by CopyBitmap / InvertBitmap) by aligning the destination bit offset up-front so the main transfer loop can use a writer that assumes byte alignment, reducing per-word split/write overhead for unaligned cases.

Changes:

• Added a shared TransferReaderWriter helper to consolidate the word/trailing-byte transfer loops.
• Updated TransferBitmap to copy a small prefix when dest_offset is bit-unaligned, making dest_offset byte-aligned for the remainder of the transfer.
• Switched the main unaligned-read path to use BitmapWordWriter<uint64_t, /*may_have_byte_offset=*/false> once the destination is aligned.

[AntoinePrv]

@ursabot please benchmark lang=C++

[rok]

Benchmark runs are scheduled for commit 10c08f4. 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.

[conbench-apache-arrow]

Thanks for your patience. Conbench analyzed the 3 benchmarking runs that have been run so far on PR commit 10c08f4.

There were 8 benchmark results indicating a performance regression:

• Pull Request Run on amd64-c6a-4xlarge-linux at 2026-08-03 16:55:16Z
  • CopyBitmapWithoutOffset (C++) with params=8192, source=cpp-micro, suite=arrow-bit-util-benchmark
  • BitRunReader (C++) with params=-1, source=cpp-micro, suite=arrow-bit-util-benchmark
• and 6 more (see the report linked below)

The full Conbench report has more details.

[conbench-apache-arrow]

Thanks for your patience. Conbench analyzed the 3 benchmarking runs that have been run so far on PR commit 10c08f4.

There were 8 benchmark results indicating a performance regression:

• Pull Request Run on amd64-c6a-4xlarge-linux at 2026-08-03 16:55:16Z
  • CopyBitmapWithoutOffset (C++) with params=8192, source=cpp-micro, suite=arrow-bit-util-benchmark
  • BitRunReader (C++) with params=-1, source=cpp-micro, suite=arrow-bit-util-benchmark
• and 6 more (see the report linked below)

The full Conbench report has more details.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

cpp/src/arrow/util/bitmap_ops.cc:229

• In the generic (non-identity) branch, last_data is computed using ~data[...] instead of the provided op. This happens to work for std::bit_not<>, but it makes the template internally inconsistent and would be incorrect if MapBitmapUnary were reused with any other unary op.

      last_data = ~data[num_bytes - 1];

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

cpp/src/arrow/util/bitmap_ops.cc:238

• MapBitmapUnary applies Op to all full bytes but hard-codes last_data = ~data[...] for the final (possibly partial) byte. This makes the template incorrect for any Op other than bitwise-not and breaks the stated generic behavior.

      constexpr auto op = Op{};
      for (int64_t i = 0; i < num_bytes - 1; i++) {
        dest[i] = static_cast<uint8_t>(op(data[i]));
      }
      last_data = op(data[num_bytes - 1]);

cpp/src/arrow/util/bitmap_ops.cc:179

• Docstring typos/wording: “sace” -> “save”; “non bit-aligned input and outputs” -> “non-bit-aligned inputs and outputs”; and this code is aligning to a byte boundary, so “bit-align the writer” is misleading.

/// Map inputs with a given operation and sace to output.
///
/// This function assumes general non bit-aligned input and outputs.
/// It will first process less than a byte in order to bit-align the writer, and then
/// keep on going with an aligned writer.

cpp/src/arrow/util/bitmap_ops.cc:130

• Docstring grammar: “All readers and writer” should read “All readers and the writer”, and “as many input” should be “as many inputs”.

This issue also appears on line 175 of the same file.

/// Map output from readers and save it with the writer.
///
/// All readers and writer must span over the same number of values.
///
/// @tparam Op a function of as many input as there are readers.

cpp/src/arrow/util/bitmap_ops.cc:169

• BitmapPtr::operator+ doesn’t mutate state and should be const so it can be used on const-qualified instances (and better reflects intent).

This issue also appears on line 234 of the same file.

  BitmapPtr operator+(int64_t extra) { return {.data = data, .offset = offset + extra}; }

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

cpp/src/arrow/util/bitmap_ops.cc:175

• Typo in the new doc comment: "sace" should be "save".

/// Map inputs with a given operation and sace to output.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

cpp/src/arrow/util/bitmap_ops.cc:175

• Typo in doc comment: "sace" should be "save".

/// Map inputs with a given operation and sace to output.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

cpp/src/arrow/util/bitmap_ops.cc:175

• Typo in doc comment: "sace" should be "save".

/// Map inputs with a given operation and sace to output.

cpp/src/arrow/util/bitmap_ops.cc:169

• BitmapPtr::operator+ isn't const, which prevents using it with const BitmapPtr values (and makes it harder to reuse this helper safely). Making it const (and optionally constexpr) keeps the API flexible without changing behavior.

  BitmapPtr operator+(int64_t extra) { return {.data = data, .offset = offset + extra}; }

[AntoinePrv]

@ursabot please benchmark lang=C++

[rok]

Benchmark runs are scheduled for commit 5f47de7. 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.

[conbench-apache-arrow]

Thanks for your patience. Conbench analyzed the 4 benchmarking runs that have been run so far on PR commit 5f47de7.

There were 8 benchmark results indicating a performance regression:

• Pull Request Run on amd64-c6a-4xlarge-linux at 2026-08-04 16:38:39Z
  • BM_UnpackUint32 (C++) with params=ScalarUnaligned/8/32, source=cpp-micro, suite=arrow-bpacking-benchmark
  • BM_UnpackUint8 (C++) with params=DynamicUnaligned/8/32, source=cpp-micro, suite=arrow-bpacking-benchmark
• and 6 more (see the report linked below)

The full Conbench report has more details.

[AntoinePrv]

These benchmark results make no sense here.

I think we lost the initial CopyBitmapWithoutOffset regression which could have been explained by the way I reordered if clauses.

Locally, I see great improvements (~80% now) for bitmap unary operations (Copy/Invert) when both are offset.
I think the path that get optimized for binary operations does not get exercised by the current benchmarks.

What do you think of these changes @cyb70289, seems you were among the last involved here.

[cyb70289]

Nice improvement, LGTM.