-
Notifications
You must be signed in to change notification settings - Fork 276
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
perf(encoding): speed up EncodeVarint with io.ByteWriter+hand rolled varintEncode #917
perf(encoding): speed up EncodeVarint with io.ByteWriter+hand rolled varintEncode #917
Conversation
…varintEncode This change speeds up EncodeVarint by testing if the input writer implements io.ByteWriter and if so, goes to use our hand-rolled varint encoder, instead of using the awkward standard libary encoding/binary.PutVarint that requires a byteslice, which we also retrofitted using a bytearray pool. While here, added parity tests to ensure that we get the exact same results as with the Go standard library's encoding/binary package with caution from https://cyber.orijtech.com/advisory/varint-decode-limitless and also added benchmarks whose results reflect the change in just the benchmark initially ```shell $ benchstat before.txt after.txt name old time/op new time/op delta EncodeVarint-8 360ns ± 3% 245ns ± 3% -31.80% (p=0.000 n=10+10) name old alloc/op new alloc/op delta EncodeVarint-8 0.00B 0.00B ~ (all equal) name old allocs/op new allocs/op delta EncodeVarint-8 0.00 0.00 ~ (all equal) ``` Fixes cosmos#891
WalkthroughThe overall change focuses on optimizing the encoding of variable integers by introducing a more efficient handling mechanism when writing to Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
9679c03
to
0708488
Compare
Kindly cc-ing @ValarDragon |
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.
Ah amazing, nice job!
@Mergifyio backport release/v1.x |
❌ No backport have been created
GitHub error: |
@Mergifyio backport release/v1.1.x |
@Mergifyio backport release/v1.x.x |
✅ Backports have been created
|
✅ Backports have been created
|
…varintEncode (backport #917) (#918) Co-authored-by: Emmanuel T Odeke <[email protected]>
Thanks for the reviews Dev & Marko! |
…varintEncode (backport #917) (#919) Co-authored-by: Emmanuel T Odeke <[email protected]>
This change speeds up EncodeVarint by testing if the input writer implements io.ByteWriter and if so, goes to use our hand-rolled varint encoder, instead of using the awkward standard libary encoding/binary.PutVarint that requires a byteslice, which we also retrofitted using a bytearray pool.
While here, added parity tests to ensure that we get the exact same results as with the Go standard library's encoding/binary package with caution from https://cyber.orijtech.com/advisory/varint-decode-limitless and also added benchmarks whose results reflect the change in just the benchmark initially
Fixes #891