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

Remove an allocation when encoding messages #396

Merged
merged 3 commits into from
Oct 22, 2015
Merged

Remove an allocation when encoding messages #396

merged 3 commits into from
Oct 22, 2015

Conversation

nussjustin
Copy link
Contributor

The old code for encode used a bytes.Buffer, causing an unnecessary allocation. The new code allocates a raw []byte with the proper length and writes directly into it.

benchmark                   old ns/op     new ns/op     delta
BenchmarkEncode1B-8         1119          1060          -5.27%
BenchmarkEncode1KiB-8       3455          2104          -39.10%
BenchmarkEncode8KiB-8       10986         10266         -6.55%
BenchmarkEncode64KiB-8      83889         76763         -8.49%
BenchmarkEncode512KiB-8     719323        656051        -8.80%
BenchmarkEncode1MiB-8       1499583       1466814       -2.19%

benchmark                   old MB/s     new MB/s     speedup
BenchmarkEncode1B-8         7.15         7.54         1.05x
BenchmarkEncode1KiB-8       298.63       490.41       1.64x
BenchmarkEncode8KiB-8       746.34       798.70       1.07x
BenchmarkEncode64KiB-8      781.32       853.86       1.09x
BenchmarkEncode512KiB-8     728.88       799.17       1.10x
BenchmarkEncode1MiB-8       699.25       714.87       1.02x

benchmark                   old allocs     new allocs     delta
BenchmarkEncode1B-8         3              3              +0.00%
BenchmarkEncode1KiB-8       5              4              -20.00%
BenchmarkEncode8KiB-8       5              4              -20.00%
BenchmarkEncode64KiB-8      5              4              -20.00%
BenchmarkEncode512KiB-8     5              4              -20.00%
BenchmarkEncode1MiB-8       6              5              -16.67%

benchmark                   old bytes     new bytes     delta
BenchmarkEncode1B-8         336           240           -28.57%
BenchmarkEncode1KiB-8       2768          2528          -8.67%
BenchmarkEncode8KiB-8       17232         17120         -0.65%
BenchmarkEncode64KiB-8      147797        147684        -0.08%
BenchmarkEncode512KiB-8     1065336       1065217       -0.01%
BenchmarkEncode1MiB-8       2113965       2113848       -0.01%

@iamqizhao
Copy link
Contributor

Please sync.

@nussjustin
Copy link
Contributor Author

Done, rebased changes

iamqizhao
iamqizhao reviewed Oct 21, 2015
View reviewed changes
rpc_util.go
@@ -172,16 +168,27 @@ func encode(c Codec, msg interface{}, pf payloadFormat) ([]byte, error) {
if err != nil {
return nil, err
}
length = uint(len(b))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentionally. Please keep uint (note that len(...) returns int). uint32 makes the overflow checking in line#177 void.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

iamqizhao added a commit that referenced this pull request Oct 22, 2015
@iamqizhao
Merge pull request #396 from nuss-justin/master
1f1a499 
Remove an allocation when encoding messages
@iamqizhao iamqizhao merged commit 1f1a499 into grpc:master Oct 22, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nussjustin @iamqizhao