Skip to content

perf(arrow/array): bulk append BinaryView nulls - #1232

Merged
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:perf/arrow-binaryview-append-nulls
Aug 28, 2026
Merged

perf(arrow/array): bulk append BinaryView nulls#1232
zeroshade merged 1 commit into
apache:mainfrom
fallintoplace:perf/arrow-binaryview-append-nulls

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Replace the per-value validity updates in BinaryViewBuilder.AppendNulls with one bulk bitmap update.
  • Update the length and null count once per batch.
  • Treat zero and negative counts as no-ops.
  • Add parity coverage for aligned and unaligned starting positions.

Benchmark

Apple M1 Pro, Go 1.26.3. The builder is preallocated and reused, so this measures the bitmap append itself. Medians from 5 runs at offset 0:

Command: go test -vet=off ./arrow/array -run "^$" -bench "^BenchmarkBinaryViewBuilderAppendNulls$" -benchmem -benchtime=500ms -count=5 -cpu=1

Values Before After
1,024 764.8 ns/op 19.66 ns/op
65,536 40,266 ns/op 152.2 ns/op

Both versions use 0 B/op and 0 allocs/op in this preallocated benchmark.

Tests

  • go test ./arrow/array -count=1
  • PARQUET_TEST_DATA=parquet-testing/data go test ./... -count=1

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bitmap optimization and performance improvement are reproducible, and normal parity, race, and allocation checks pass. However, the new bulk path can silently corrupt builder state when the requested count overflows its length arithmetic.

return
}

b.Reserve(n)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reject counts that would overflow b.length + n before calling Reserve.

With one existing value, AppendNulls(math.MaxInt) now returns with Len() == math.MinInt and NullN() == math.MaxInt. NewBinaryViewArray() then produces a negative-length array whose ValidateFull() still returns nil. A guard such as n > math.MaxInt-b.length, plus boundary regression coverage, would prevent this silent corruption.

@fallintoplace
fallintoplace force-pushed the perf/arrow-binaryview-append-nulls branch from edfbeef to 616998d Compare August 28, 2026 18:11
@zeroshade
zeroshade merged commit 58995de into apache:main Aug 28, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants