You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/09/11 11:00:34 UTC
[PR] perf(arrow/array): append dictionary indices directly [arrow-go]
fallintoplace opened a new pull request, #1313:
URL: https://github.com/apache/arrow-go/pull/1313
## What does this change?
- `DictionaryBuilder.AppendIndices` used to allocate a new typed slice on every call.
- Convert the input values directly into the final typed index buffer.
- Keep the existing bulk validity bitmap path.
- Add coverage for all eight index types, nullable appends, split appends, and invalid validity lengths.
- No public API changes.
## Benchmark
Command:
```
go test ./arrow/array -run '^$' -bench '^BenchmarkDictionaryBuilderAppendIndices$' -benchmem -benchtime=300ms -count=8
```
Apple M1 Pro, Go 1.26.3. The benchmark uses 65,536 indices and a 256-entry dictionary.
| Case | Before ns/op | After ns/op | Before B/op | After B/op | Allocs |
| --- | ---: | ---: | ---: | ---: | ---: |
| int32, non-null | 110,370 | 75,398 | 815,908 | 553,762 | 18 -> 17 |
| int32, nullable | 116,578 | 75,895 | 815,907 | 553,762 | 18 -> 17 |
This is about 32% less time and allocation bytes for the non-null case, and about 35% less time for the nullable case.
The Parquet decoder's separate `[]uint64 -> []int` scratch conversion is intentionally outside this PR.
## Tests
- `go test ./...`
- `go test -race ./arrow/array`
- `go vet ./arrow/array`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(arrow/array): append dictionary indices directly [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #1313:
URL: https://github.com/apache/arrow-go/pull/1313#issuecomment-5686705655
Generally looks good but needs to have the lint issue fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(arrow/array): append dictionary indices directly [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1313:
URL: https://github.com/apache/arrow-go/pull/1313#discussion_r4018769487
##########
arrow/array/dictionary_append_indices_benchmark_test.go:
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package array_test
+
+import (
+ "fmt"
+ "strconv"
+ "testing"
+
+ "github.com/apache/arrow-go/v18/arrow"
+ "github.com/apache/arrow-go/v18/arrow/array"
+ "github.com/apache/arrow-go/v18/arrow/memory"
+)
+
+func BenchmarkDictionaryBuilderAppendIndices(b *testing.B) {
+ const (
+ length = 1 << 16
+ cardinality = 1 << 8
Review Comment:
Non-blocking: the shared workload uses 256 distinct indices, but the int8 case cannot represent indices 128–255; they narrow to negative dictionary indices. Could we cap the shared cardinality at 128, or choose a valid cardinality per index type, so each benchmark materializes a valid dictionary array?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(arrow/array): append dictionary indices directly [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1313:
URL: https://github.com/apache/arrow-go/pull/1313
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org