You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "hamilton-earthscope (via GitHub)" <gi...@apache.org> on 2025/10/30 05:44:21 UTC
[PR] perf: prealloc data builder with estimated total output size [arrow-go]
hamilton-earthscope opened a new pull request, #557:
URL: https://github.com/apache/arrow-go/pull/557
### Rationale for this change
When writing data to partitioned Iceberg tables using `iceberg-go`'s new partitioned write capability, I found that Arrow's `compute.Take` operation was causing the writes to be significantly slower than unpartitioned writes.
Cause: the `VarBinaryImpl` function here in `arrow-go` was pre-allocating the data buffer with only `meanValueLen` (size for just one string/buffer), not the total size needed for all strings. For, my use case (writing 100k rows at a time) this was causing 15x buffer reallocations as the buffer incrementally scales up. For my iceberg table with 20+ string/binary cols of various size this is significant overhead.
### What changes are included in this PR?
Pre-allocate upfront with a better estimate of the necessary buffer size to eliminate repeated reallocations.
I somewhat arbitrarily chose a cap of 16MB as a guess at what would be effective at reducing the number of allocations but also trying to be cognizant of the library being used in many bespoke scenarios and not wanting to make massive memory spikes. For my use case, I never hit this 16MB threshold and it could be smaller.
I am curious for your input on whether there should be a cap at all or what a reasonable cap would be.
### Are these changes tested?
No unit tests.
However, for my use case - writing 100k rows to a partitioned iceberg table:
- before: `table.Append()` took 30s on average
- after: `table.Append()` takes 2.5s on average
### Are there any user-facing changes?
No; just more performant
--
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: optimize compute.Take for fewer memory allocations [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #557:
URL: https://github.com/apache/arrow-go/pull/557
--
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: optimize compute.Take for fewer memory allocations [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #557:
URL: https://github.com/apache/arrow-go/pull/557#discussion_r2478529987
##########
arrow/compute/internal/kernels/vector_selection.go:
##########
@@ -1486,7 +1486,15 @@ func VarBinaryImpl[OffsetT int32 | int64](ctx *exec.KernelCtx, batch *exec.ExecS
if values.Len > 0 {
dataLength := rawOffsets[values.Len] - rawOffsets[0]
meanValueLen := float64(dataLength) / float64(values.Len)
- dataBuilder.reserve(int(meanValueLen))
+ estimatedTotalSize := int(meanValueLen * float64(outputLength))
+
+ // Cap the pre-allocation at a reasonable size
+ const maxPreAlloc = 16777216 // 16 MB
+ if estimatedTotalSize > maxPreAlloc {
+ estimatedTotalSize = maxPreAlloc
+ }
Review Comment:
```suggestion
estimatedTotalSize = min(maxPreAlloc, estimatedTotalSize)
```
--
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: optimize compute.Take for fewer memory allocations [arrow-go]
Posted by "hamilton-earthscope (via GitHub)" <gi...@apache.org>.
hamilton-earthscope commented on PR #557:
URL: https://github.com/apache/arrow-go/pull/557#issuecomment-3468894108
on `main`:
```
go test -bench=BenchmarkTakeString -benchmem -benchtime=3x -run=^$ -timeout 30m
goos: darwin
goarch: arm64
pkg: github.com/apache/arrow-go/v18/arrow/compute
cpu: Apple M3 Max
BenchmarkTakeString/SmallBatch_ShortStrings-16 3 139750 ns/op 7163478 rows/sec 835890 B/op 198 allocs/op
BenchmarkTakeString/MediumBatch_ShortStrings-16 3 13052111 ns/op 766200 rows/sec 98296896 B/op 1762 allocs/op
BenchmarkTakeString/LargeBatch_ShortStrings-16 3 193090167 ns/op 258947 rows/sec 2746571538 B/op 9389 allocs/op
BenchmarkTakeString/XLargeBatch_ShortStrings-16 3 564450403 ns/op 177164 rows/sec 11119508282 B/op 18947 allocs/op
BenchmarkTakeString/SmallBatch_MediumStrings-16 3 3269042 ns/op 305968 rows/sec 21126634 B/op 826 allocs/op
BenchmarkTakeString/MediumBatch_MediumStrings-16 3 198658722 ns/op 50338 rows/sec 2055054277 B/op 8127 allocs/op
BenchmarkTakeString/LargeBatch_MediumStrings-16 3 2286152306 ns/op 21871 rows/sec 52530632896 B/op 41554 allocs/op
BenchmarkTakeString/XLargeBatch_MediumStrings-16 3 8208929458 ns/op 12182 rows/sec 210687783749 B/op 83644 allocs/op
BenchmarkTakeString/LargeBatch_ShortStrings_Large-16 3 183346069 ns/op 272709 rows/sec 2746760874 B/op 9312 allocs/op
BenchmarkTakeString/XLargeBatch_MediumStrings_Large-16 3 8180564000 ns/op 12224 rows/sec 210688177797 B/op 83568 allocs/op
BenchmarkTakeStringPartitionPattern-16 3 378207430 ns/op 132203 rows/sec 6315631330 B/op 14193 allocs/op
PASS
ok github.com/apache/arrow-go/v18/arrow/compute 81.473s
```
this PR:
```
go test -bench=BenchmarkTakeString -benchmem -benchtime=3x -run=^$ -timeout 30m
goos: darwin
goarch: arm64
pkg: github.com/apache/arrow-go/v18/arrow/compute
cpu: Apple M3 Max
BenchmarkTakeString/SmallBatch_ShortStrings-16 3 42167 ns/op 23920202 rows/sec 17906 B/op 44 allocs/op
BenchmarkTakeString/MediumBatch_ShortStrings-16 3 156903 ns/op 63869468 rows/sec 160456 B/op 44 allocs/op
BenchmarkTakeString/LargeBatch_ShortStrings-16 3 760750 ns/op 65783472 rows/sec 805077 B/op 43 allocs/op
BenchmarkTakeString/XLargeBatch_ShortStrings-16 3 1148056 ns/op 87144906 rows/sec 1614917 B/op 43 allocs/op
BenchmarkTakeString/SmallBatch_MediumStrings-16 3 29931 ns/op 33707865 rows/sec 64749 B/op 42 allocs/op
BenchmarkTakeString/MediumBatch_MediumStrings-16 3 229431 ns/op 43689299 rows/sec 562298 B/op 42 allocs/op
BenchmarkTakeString/LargeBatch_MediumStrings-16 3 705195 ns/op 70951339 rows/sec 2812080 B/op 42 allocs/op
BenchmarkTakeString/XLargeBatch_MediumStrings-16 3 1352417 ns/op 73981205 rows/sec 5612592 B/op 42 allocs/op
BenchmarkTakeString/LargeBatch_ShortStrings_Large-16 3 571167 ns/op 87591241 rows/sec 1001650 B/op 42 allocs/op
BenchmarkTakeString/XLargeBatch_MediumStrings_Large-16 3 1353181 ns/op 73933345 rows/sec 6014005 B/op 42 allocs/op
BenchmarkTakeStringPartitionPattern-16 3 650014 ns/op 76983953 rows/sec 1312954 B/op 42 allocs/op
PASS
ok github.com/apache/arrow-go/v18/arrow/compute 0.585s
```
--
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