From 757e898e5f4e83bd158916a7e479bad6388bd811 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Mon, 25 Oct 2021 09:33:09 -0400 Subject: [PATCH] bench: fix colserde benchmarks And optimization to aid the Go GC broke benchmarks of Serialize and ArrowToBench, fix them by restoring the nil'd out arrays using a copy. Fixes: #71886 Release note: None --- pkg/col/colserde/arrowbatchconverter_test.go | 6 +++++- pkg/col/colserde/record_batch_test.go | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/col/colserde/arrowbatchconverter_test.go b/pkg/col/colserde/arrowbatchconverter_test.go index 06a8cee258dc..2cbea434a321 100644 --- a/pkg/col/colserde/arrowbatchconverter_test.go +++ b/pkg/col/colserde/arrowbatchconverter_test.go @@ -224,15 +224,19 @@ func BenchmarkArrowBatchConverter(b *testing.B) { for _, nullFraction := range nullFractions { setNullFraction(batch, nullFraction) data, err := c.BatchToArrow(batch) + dataCopy := make([]*array.Data, len(data)) require.NoError(b, err) testPrefix := fmt.Sprintf("%s/nullFraction=%0.2f", typ.String(), nullFraction) result := testAllocator.NewMemBatchWithMaxCapacity([]*types.T{typ}) b.Run(testPrefix+"/ArrowToBatch", func(b *testing.B) { b.SetBytes(numBytes[typIdx]) for i := 0; i < b.N; i++ { + // Since ArrowToBatch eagerly nils things out, we have to make a + // shallow copy each time. + copy(dataCopy, data) // Using require.NoError here causes large enough allocations to // affect the result. - if err := c.ArrowToBatch(data, batch.Length(), result); err != nil { + if err := c.ArrowToBatch(dataCopy, batch.Length(), result); err != nil { b.Fatal(err) } if result.Width() != 1 { diff --git a/pkg/col/colserde/record_batch_test.go b/pkg/col/colserde/record_batch_test.go index 3dd8db03babe..37633b285ce7 100644 --- a/pkg/col/colserde/record_batch_test.go +++ b/pkg/col/colserde/record_batch_test.go @@ -369,11 +369,15 @@ func BenchmarkRecordBatchSerializerInt64(b *testing.B) { // Only calculate useful bytes. numBytes := int64(dataLen * 8) data := []*array.Data{randomDataFromType(rng, typs[0], dataLen, 0 /* nullProbability */)} + dataCopy := make([]*array.Data, len(data)) b.Run(fmt.Sprintf("Serialize/dataLen=%d", dataLen), func(b *testing.B) { b.SetBytes(numBytes) for i := 0; i < b.N; i++ { buf.Reset() - if _, _, err := s.Serialize(&buf, data, dataLen); err != nil { + // Since Serialize eagerly nils things out, we have to make a shallow + // copy each time. + copy(dataCopy, data) + if _, _, err := s.Serialize(&buf, dataCopy, dataLen); err != nil { b.Fatal(err) } }