Skip to content

Commit b29cea5

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 maintain the malformed message protection by using a 10 MiB buffer for larger asks. The existing benchmarks show that we reduce allocations for small payloads, and keep the edge case on a reasonable level at about 10MiB. 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% BenchmarkSafeReadBytes/max-askedSize-20 8 3 -62.50% 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% BenchmarkSafeReadBytes/max-askedSize-20 15992 10489910 +65494.73% 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 7ec4177 commit b29cea5

File tree

1 file changed

+24
-3
lines changed

1 file changed

+24
-3
lines changed

lib/go/thrift/binary_protocol.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,28 @@ func safeReadBytes(size int32, trans io.Reader) ([]byte, error) {
555555
return nil, nil
556556
}
557557

558-
buf := new(bytes.Buffer)
559-
_, err := io.CopyN(buf, trans, int64(size))
560-
return buf.Bytes(), err
558+
const readLimit = 10 * 1024 * 1024
559+
560+
if size < readLimit {
561+
b := make([]byte, size)
562+
n, err := io.ReadFull(trans, b)
563+
return b[:n], err
564+
}
565+
566+
b := make([]byte, readLimit)
567+
var buf bytes.Buffer
568+
var e error
569+
var n int
570+
for size > 0 {
571+
n, e = io.ReadFull(trans, b)
572+
buf.Write(b[:n])
573+
if e != nil {
574+
break
575+
}
576+
size -= readLimit
577+
if size < readLimit && size > 0 {
578+
b = b[:size]
579+
}
580+
}
581+
return buf.Bytes(), e
561582
}

0 commit comments

Comments
 (0)