googleapis / googleapis/google-cloud-go

bigquery: Performance when parsing NUMERIC values from Arrow could be improved

Open
#12,865 2 comments 0 reactions 1 assignee Claimed by @alvarowolfx View on GitHub
api: bigquery priority: p2
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

Image

Allocated space

Image

## 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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.