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

perf(encoding): speed up EncodeVarint with io.ByteWriter+hand rolled varintEncode (backport #917) #918

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Mar 15, 2024

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

$ 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 #891


This is an automatic backport of pull request #917 done by Mergify.

@tac0turtle tac0turtle enabled auto-merge (squash) March 15, 2024 10:37
@tac0turtle tac0turtle merged commit b4fcc41 into release/v1.1.x Mar 15, 2024
5 checks passed
@tac0turtle tac0turtle deleted the mergify/bp/release/v1.1.x/pr-917 branch March 15, 2024 10:44
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