perf(arrow/array): skip full-range SliceEqual slices - #1249
perf(arrow/array): skip full-range SliceEqual slices#1249fallintoplace wants to merge 3 commits into
Conversation
zeroshade
left a comment
There was a problem hiding this comment.
Skipping slice construction for full-range inputs changes compatibility with arbitrary arrow.Array implementations.
The previous path normalized inputs through NewSlice/MakeFromData. The new fast path calls Equal directly, which panics for valid wrapper implementations it does not recognize. I reproduced this with a wrapper around *array.Int64; the same issue propagates through one-chunk ChunkedEqual. Details are inline.
This head currently has no CI results.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Arrow Go maintainer. After you've
addressed the points above and pushed an update, an Apache Arrow Go
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.More on how Apache Arrow Go handles maintainer review:
CONTRIBUTING.md.
|
|
||
| // SliceEqual reports whether slices left[lbeg:lend] and right[rbeg:rend] are equal. | ||
| func SliceEqual(left arrow.Array, lbeg, lend int64, right arrow.Array, rbeg, rend int64) bool { | ||
| if lbeg == 0 && lend == int64(left.Len()) && rbeg == 0 && rend == int64(right.Len()) { |
There was a problem hiding this comment.
The full-range fast path calls Equal on the original interface values. Equal only handles Arrow-Go’s concrete array types or ExtensionArray, so another valid arrow.Array implementation now panics. Wrapping an *array.Int64 in struct{ arrow.Array } and calling full-range SliceEqual previously returned true; this head panics with arrow/array: unknown array type. Please retain the slice-normalization path for unsupported implementations or make Equal safely normalize generic arrays, and add the wrapper regression test.
Summary
NewSlicepath for partial ranges.ChunkedEqualwhen chunk boundaries line up.Benchmark
Local Apple M1 Pro.
-benchtime=500ms -count=3.SliceEqualint64, 64 valuesSliceEqualstring, 64 valuesChunkedEqual, 1,024 chunks x 64 valuesTests
go test ./arrow/arraygo test -race ./arrow/arraygo vet -composites=false ./arrow/arraygo test ./... -run '^$'GOOS=linux GOARCH=386 go test -c -o /dev/null ./arrow/array