Skip to content

perf(arrow/array): compare approximate list values by valid runs - #1330

Open
fallintoplace wants to merge 8 commits into
apache:mainfrom
fallintoplace:perf-approx-list-valid-runs
Open

fallintoplace wants to merge 8 commits into
apache:mainfrom
fallintoplace:perf-approx-list-valid-runs

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

What

Speed up ApproxEqual for list arrays.

Why

The old path created and released one child slice for every valid list item. This is expensive for large arrays.

Implementation

  • Compare contiguous valid runs for List, LargeList, and FixedSizeList.
  • Reuse direct child comparison when a run covers the full child array.
  • Keep concrete scalar fallbacks for fragmented validity bitmaps.
  • Add coverage for full-range approximate slices and generic arrays.
  • Add benchmark coverage for common null patterns.

Benchmark

Apple M1 Pro, Go 1.26.3, GOMAXPROCS=1, 65,536 rows, Int32 child values, list size 16. Median of 3 runs. Lower ns/op is better.

Case upstream/main This PR Speed PR allocs/op
list / all-valid 17.8 ms 0.197 ms 90.5x 2
large-list / all-valid 18.9 ms 0.216 ms 87.8x 2
fixed-size-list / all-valid 18.0 ms 0.145 ms 124x 2
list / 10% null 16.2 ms 2.15 ms 7.5x 26,218
list / clustered 10% null 16.0 ms 0.195 ms 81.9x 10
list / alternating null 8.72 ms 8.71 ms 1.00x, near parity 131,074

@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 new full-range sliceApproxEqual fast path bypasses DataType normalization and changes public behavior.

array.NewBinaryBuilder(mem, arrow.BinaryTypes.String) produces concrete *array.Binary values carrying STRING data types. For two such operands, canEqualDirectly accepts the matching concrete types and the shortcut dispatches to exact binary comparison rather than string approximate comparison. Consequently "a\\x00" versus "a" changes from true to false; LargeString has the analogous issue.

I reproduced this against current head 89944d11: the full-range STRING comparison loses the established trailing-NUL normalization. The previous NewSlice/MakeFromData path normalized these values to *String. Please restrict the shortcut where concrete type and logical DataType semantics agree (or retain the normalization path for these ambiguous Binary/String representations) and add this builder-produced case.

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