Closed hicqu closed 3 years ago
LGTM
@jayzhan211 thank you! I have added unit test cases for the new implementation. Could you take a look again and approve it?
@jayzhan211 or could you push more commits to #423 ? Your implementation involves less allocation and free. I can compare them later.
@jayzhan211 or could you push more commits to #423 ?
What commits do you mean?
Here is a benchmark result: for master branch:
Inflights(256) time: [2.1513 us 2.2001 us 2.2609 us]
Inflights(4096) time: [35.550 us 36.232 us 36.976 us]
for the patch:
Inflights(256) time: [6.8643 us 7.1716 us 7.5381 us]
change: [+205.14% +217.31% +228.40%] (p = 0.00 < 0.05)
Performance has regressed.
Inflights(4096) time: [103.92 us 108.21 us 113.07 us]
change: [+192.46% +201.84% +211.64%] (p = 0.00 < 0.05)
Performance has regressed.
Signed-off-by: qupeng qupeng@pingcap.com
Like #423 this PR also want to resolve #419 , but use another solution that lazy allocating inflight buffers and shrink it if necessary. No new APIs are introduced.
Would you mind to take a look? @jayzhan211