googleapis / googleapis/google-cloud-go
bigquery: Performance when parsing NUMERIC values from Arrow could be improved
- Dominant language
- Go
- Stars
- 4.5k
- Forks
- 1.6k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 109
Description
## Client
BigQuery Storage Read Client
## Environment
- Go version: Go 1.24
- Platform: macOS
- Package: `cloud.google.com/go/bigquery`
## Code and Dependencies
(see below for more context) The test below benchmarks the previous implementation (with a branch to handle the negative case) and the current (string-based) implementation introduced in #11052.
Run with `go test -bench .`
```go
package bigquery_arrow_deserialise_memory
import (
"math/big"
"testing"
"github.com/apache/arrow/go/v15/arrow"
"github.com/apache/arrow/go/v15/arrow/array"
"github.com/apache/arrow/go/v15/arrow/decimal128"
"github.com/apache/arrow/go/v15/arrow/memory"
)
const (
precision = 32
scale = 9
)
// buildArray creates a Decimal128 array with a single value
func buildArray(t testing.TB) (*arrow.Decimal128Type, *array.Decimal128) {
t.Helper()
dft := &arrow.Decimal128Type{Precision: precision, Scale: scale}
arr := array.NewDecimal128Builder(memory.DefaultAllocator, dft)
dec, err := decimal128.FromFloat64(-123456789.123456789, precision, scale)
if err != nil {
t.Fatal(err)
}
arr.Append(dec)
col := arr.NewDecimal128Array()
return dft, col
}
// Taken from https://github.com/googleapis/google-cloud-go/blob/a76dedc161bb687ea235f14045f60edd52953270/bigquery/arrow.go#L241-L246
func CurrentDeserialise128(col *array.Decimal128, i int, dft *arrow.Decimal128Type) *big.Rat {
v := col.Value(i)
rat := new(big.Rat)
rat.SetString(v.ToString(dft.Scale))
return rat
}
// See: https://github.com/googleapis/google-cloud-go/pull/11052/files#diff-4d28364acf2bcbdb09526ea1b3cf4ee065377af9d9103b0432de3c17b0104ff4L242-L248
// Modified to correctly handle negative values
func PreviousDeserialise128WithNegativeCheck(col *array.Decimal128, i int, dft *arrow.Decimal128Type) *big.Rat {
v := col.Value(i)
rat := big.NewRat(1, 1)
rat.Num().SetBytes(v.BigInt().Bytes())
d := rat.Denom()
d.Exp(big.NewInt(10), big.NewInt(int64(dft.Scale)), nil)
if v.Sign() < 0 {
rat.Neg(rat)
}
return rat
}
func runBenchmark(b *testing.B, fn func(col *array.Decimal128, i int, dft *arrow.Decimal128Type) *big.Rat) {
dft, col := buildArray(b)
defer col.Release()
b.ReportAllocs()
b.StartTimer()
for i := 0; i < b.N; i++ {
_ = fn(col, 0, dft)
}
b.StopTimer()
}
func BenchmarkCurrent(b *testing.B) {
runBenchmark(b, CurrentDeserialise128)
}
func BenchmarkPreviousWithNegativeCheck(b *testing.B) {
runBenchmark(b, PreviousDeserialise128WithNegativeCheck)
}
```
go.mod
```text
module github.com/tfinlay-lightspeed/bigquery_arrow_deserialise_memory
go 1.24
require github.com/apache/arrow/go/v15 v15.0.2
require (
github.com/goccy/go-json v0.10.2 // indirect
github.com/google/flatbuffers v23.5.26+incompatible // indirect
github.com/klauspost/cpuid/v2 v2.2.5 // indirect
github.com/zeebo/xxh3 v1.0.2 // indirect
golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
golang.org/x/mod v0.13.0 // indirect
golang.org/x/sys v0.13.0 // indirect
golang.org/x/tools v0.14.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
)
```
## Results
Here are the benchmark results from my machine, indicating that the previous approach is faster and allocates less memory. Over many rows, I've seen this add up to a significant difference in allocated memory.
```
goos: darwin
goarch: arm64
pkg: github.com/vend/bigquery_arrow_deserialise_memory
cpu: Apple M1 Max
BenchmarkCurrent
BenchmarkCurrent-10 913389 1309 ns/op 1016 B/op 26 allocs/op
BenchmarkPreviousWithNegativeCheck
BenchmarkPreviousWithNegativeCheck-10 5914021 211.7 ns/op 248 B/op 10 allocs/op
```
Also here is a profiler diff of parsing 100k Decimal128 values using each approach (blue is common between both approaches, green is only in current approach, and red is only in previous approach w/ negative number fix):
Allocated objects
Allocated space
## Additional context
A PR last year (#11052) fixed a bug where `NUMERIC` values were losing their sign when being parsed out of the Arrow format (thanks for the fix!). Unfortunately the updated implementation formats the value as a string and then parses it again, which impacts performance.
I hope this is helpful, happy to discuss further!
Contributor guide
Assessment
This issue has not been assessed yet.