Skip to content

Commit a258240

Browse files
committed
THRIFT-5828: reduce over-allocation in Go binary protocol
Due to a subtle caveat in buffer.Bytes the Go binary protocol method ReadBinary() always over-allocated. We now allocate the asked size directly up to 10 MiB, and then use io.CopyN to maintain the malformed message protection. The existing benchmarks show that we reduce allocations for small payloads. Ran the existing benchmarks in lib/go/thrift with and without this change: % go test -run X -bench . -benchmem > <file> % benchcmp -changed old.txt new.txt [...] benchmark old allocs new allocs delta BenchmarkSafeReadBytes/normal-20 5 2 -60.00% BenchmarkBinaryBinary_0-20 4 1 -75.00% BenchmarkBinaryBinary_1-20 4 1 -75.00% BenchmarkBinaryBinary_2-20 5 2 -60.00% BenchmarkCompactBinary0-20 4 1 -75.00% BenchmarkCompactBinary1-20 4 1 -75.00% BenchmarkCompactBinary2-20 5 2 -60.00% BenchmarkSerializer/baseline-20 8 5 -37.50% BenchmarkSerializer/plain-20 20 17 -15.00% BenchmarkSerializer/pool-20 8 5 -37.50% benchmark old bytes new bytes delta BenchmarkSafeReadBytes/normal-20 1656 160 -90.34% BenchmarkBinaryBinary_0-20 1608 160 -90.05% BenchmarkBinaryBinary_1-20 1608 160 -90.05% BenchmarkBinaryBinary_2-20 1634 184 -88.74% BenchmarkCompactBinary0-20 1608 160 -90.05% BenchmarkCompactBinary1-20 1608 160 -90.05% BenchmarkCompactBinary2-20 1634 184 -88.74% BenchmarkSerializer/baseline-20 1000 416 -58.40% BenchmarkSerializer/plain-20 3640 3056 -16.04% BenchmarkSerializer/pool-20 1002 417 -58.38%
1 parent 5cb828f commit a258240

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

lib/go/thrift/binary_protocol.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -597,14 +597,18 @@ func safeReadBytes(size int32, trans io.Reader) ([]byte, error) {
597597
if size < 0 {
598598
return nil, nil
599599
}
600-
if size > bytes.MinRead {
601-
// Use bytes.Buffer to prevent allocating size bytes when size is very large
602-
buf := new(bytes.Buffer)
603-
_, err := io.CopyN(buf, trans, int64(size))
604-
return buf.Bytes(), err
605-
}
606-
// Allocate size bytes
607-
b := make([]byte, size)
608-
n, err := io.ReadFull(trans, b)
609-
return b[:n], err
600+
601+
// Fast path for reads smaller than 10 MiB that only allocates exactly
602+
// what is asked for.
603+
const readLimit = 10 * 1024 * 1024
604+
if size < readLimit {
605+
b := make([]byte, size)
606+
n, err := io.ReadFull(trans, b)
607+
return b[:n], err
608+
}
609+
610+
// Use bytes.Buffer to prevent allocating size bytes when size is very large
611+
buf := new(bytes.Buffer)
612+
_, err := io.CopyN(buf, trans, int64(size))
613+
return buf.Bytes(), err
610614
}

0 commit comments

Comments
 (0)