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.

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

performance fix: protobuf client ioutil.ReadAll #282

Closed
wants to merge 1 commit into from
Closed

performance fix: protobuf client ioutil.ReadAll #282

wants to merge 1 commit into from

Conversation

bavovna
Copy link

@bavovna bavovna commented Oct 14, 2020

Issue #166:

ioutil.ReadAll takes 2x memory and is ~20% slower for the protobuf client. CC #166

Description of changes:

My team uses twirp as an RPC to talk to other service. Both client and server are written in go. As a part of the spec, services exchange []byte blobs. Occasionally, those blobs are fairly large in size and exceed 100M. I have detected, that ioutil.ReadAll has a noticeable performance impact for us and pretty much confirmed #166 findings.
I have created a test app and a benchmark test for it - https://github.com/mkorenkov/twirpbench. Based on the benchmark, current PR improves Total Allocations 2x and is about 20% faster, than master branch.

NOTE: requires https://github.com/gogo/protobuf/ protoc plugin. No special features are needed though - see https://github.com/mkorenkov/twirpbench/blob/master/cmd/protoc-gen-maxgo/main.go. Please let me know what you want me to do about it. I can move https://github.com/mkorenkov/twirpbench/blob/master/cmd/protoc-gen-maxgo/main.go to twirp as well and update README.md accordingly.

Benchmark results:

goos: darwin
goarch: amd64
pkg: github.com/mkorenkov/twirpbench/benchmarks
BenchmarkTwirp/twirp-raw-300K-16         	1000000000	         0.00164 ns/op	183467723440.75 MB/s	         2.64 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-raw-1M-16           	1000000000	         0.00416 ns/op	240545827347.75 MB/s	         5.99 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-raw-10M-16          	1000000000	         0.0418 ns/op	239014367846.79 MB/s	        83.1 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-raw-100M-16         	1000000000	         0.387 ns/op	258704891183.59 MB/s	       703 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-gz-300K-16          	1000000000	         0.00209 ns/op	143333898705.93 MB/s	         3.43 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-gz-1M-16            	1000000000	         0.00447 ns/op	223956391211.50 MB/s	         6.79 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-gz-10M-16           	1000000000	         0.0400 ns/op	249905885443.54 MB/s	        83.9 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/twirp-gz-100M-16          	1000000000	         0.336 ns/op	297260057441.05 MB/s	       704 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-raw-300K-16      	1000000000	         0.00139 ns/op	215784026083.97 MB/s	         1.35 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-raw-1M-16        	1000000000	         0.00302 ns/op	331665058091.13 MB/s	         3.02 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-raw-10M-16       	1000000000	         0.0330 ns/op	302895046624.03 MB/s	        41.6 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-raw-100M-16      	1000000000	         0.311 ns/op	322013349050.50 MB/s	       351 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-gz-300K-16       	1000000000	         0.00172 ns/op	174395588489.19 MB/s	         2.15 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-gz-1M-16         	1000000000	         0.00399 ns/op	250492781925.24 MB/s	         3.83 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-gz-10M-16        	1000000000	         0.0308 ns/op	324804877529.88 MB/s	        42.4 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
BenchmarkTwirp/maxtwirp-gz-100M-16       	1000000000	         0.265 ns/op	376928271748.52 MB/s	       352 TotalAlloc(MiB)	       0 B/op	       0 allocs/op
PASS
ok  	github.com/mkorenkov/twirpbench/benchmarks	26.213s

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@bavovna bavovna marked this pull request as draft October 14, 2020 20:49
@bavovna bavovna changed the title attempting to fix ioutil.ReadAll performance fix: protobuf client ioutil.ReadAll Oct 14, 2020
@bavovna bavovna marked this pull request as ready for review October 14, 2020 21:45
@amcohen-twitch
Copy link
Contributor

Hi @mkorenkov,

Thanks for your investigation. At this time, our team isn't willing to accept the tradeoff of addition of another dependency for deserialization. We'd strongly encourage pursuing performance improvements in the first party github.com/golang/protobuf library, which we will gladly take.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants