Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

THRIFT-5828: reduce over-allocation in Go binary protocol #3057

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

THRIFT-5828: reduce over-allocation in Go binary protocol #3057

wants to merge 1 commit into from

Commits on Oct 28, 2024

  1. 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%
    @leitzler
    leitzler committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    97648a2 View commit details
    Browse the repository at this point in the history