-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
transport: share recv buffers #2813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include some benchmark data in the PR description.
@@ -39,10 +40,36 @@ import ( | |||
"google.golang.org/grpc/tap" | |||
) | |||
|
|||
type bufferPool struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A notable downside of this implementation is that it consumes memory indefinitely (200 * 16KB = 3.2MB per connection). With a sync.Pool
, excess buffers are freed every GC cycle. Maybe something can slowly drain this channel to reclaim memory or maybe we should use a sync.Pool
? What was the performance of the sync.Pool
in comparison?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add benchmark results for both cases, i.e. for sync.Pool and current buffer pool implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the benchmarks posted below, it seems like sync.Pool
is better anyways. So, I switched to that.
Here are the benchmark results for the following run: for i in {1..20}
do go run benchmark/benchmain/main.go -benchtime=20s -workloads=streaming -maxConcurrentCalls=150 -reqSizeBytes=1048576 -respSizeBytes=1048576 -networkMode=Local
done Current implementation (no pool)Latency (avg 169.44684 ms)169.5530,171.5630,166.0584,172.1685,162.8318,165.5396,171.7170,166.5066,172.8472,165.5322,171.3059,178.3722,164.9176,170.7371,162.3973,172.0701,171.3668,178.8883,168.7990,165.7652 QPS (avg 17757.7)17726,17516,18089,17480,18492,18204,17513,18073,17383,18148,17543,16861,18235,17599,18505,17497,17552,16802,17808,18128 Custom bufferPool implementationLatency (avg 151.0045 ms)153.0258,142.5293,143.0294,143.2039,156.7764,159.4901,161.2431,144.4064,158.4168,153.8877,142.8829,159.3969,148.2851,152.2932,163.7162,160.7884,144.6017,146.8367,143.4129,141.8671 QPS (avg 19965.05)19621,21093,21021,21018,19170,18838,18675,20831,18961,19530,21068,18873,20294,19759,18358,18738,20769,20493,20968,21223 sync.PoolLatency (avg 147.572075 ms)151.0945,139.0543,141.9166,152.4452,148.8679,158.1198,146.4326,138.1600,145.6458,147.2584,142.4822,161.0773,142.7544,147.4883,147.2348,149.6693,143.7513,149.2043,147.7424,151.0421 QPS (avg 20403.15)19924,21646,21211,19756,20199,19026,20509,21762,20636,20390,21084,18663,21041,20420,20399,20074,20903,20147,20340,19933 |
Here are the benchmark results for the following run:
Current implementation (no pool)
Latency (avg 169.44684 ms)
169.5530,171.5630,166.0584,172.1685,162.8318,165.5396,171.7170,166.5066,172.8472,165.5322,171.3059,178.3722,164.9176,170.7371,162.3973,172.0701,171.3668,178.8883,168.7990,165.7652
QPS (avg 17757.7)
17726,17516,18089,17480,18492,18204,17513,18073,17383,18148,17543,16861,18235,17599,18505,17497,17552,16802,17808,18128
Custom bufferPool implementation
Latency (avg 151.0045 ms)
153.0258,142.5293,143.0294,143.2039,156.7764,159.4901,161.2431,144.4064,158.4168,153.8877,142.8829,159.3969,148.2851,152.2932,163.7162,160.7884,144.6017,146.8367,143.4129,141.8671
QPS (avg 19965.05)
19621,21093,21021,21018,19170,18838,18675,20831,18961,19530,21068,18873,20294,19759,18358,18738,20769,20493,20968,21223
sync.Pool
Latency (avg 147.572075 ms)
151.0945,139.0543,141.9166,152.4452,148.8679,158.1198,146.4326,138.1600,145.6458,147.2584,142.4822,161.0773,142.7544,147.4883,147.2348,149.6693,143.7513,149.2043,147.7424,151.0421
QPS (avg 20403.15)
19924,21646,21211,19756,20199,19026,20509,21762,20636,20390,21084,18663,21041,20420,20399,20074,20903,20147,20340,19933