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

opt: refactor streaming decoder to fully use buffer #550

Merged
merged 5 commits into from
Nov 7, 2023
Merged

Conversation

AsterDY
Copy link
Collaborator

@AsterDY AsterDY commented Nov 6, 2023

Optimization

  • refactor implementation of StreamDecoder.Decode()
  • reduce memory copy by limiting start and end of slice passed to skip()
  • never move head of decoder buffer, always move tail
  • check if buffer remains valid bytes after eachDecode() or More(), if not, recycle buffer

Benchmark

name                          old time/op    new time/op    delta
DecodeStream_Sonic/halt-16       698µs ± 7%     732µs ± 3%   +4.84%  (p=0.002 n=10+10)
DecodeStream_Sonic/single-16     273µs ± 7%     191µs ± 4%  -30.03%  (p=0.000 n=9+10)
DecodeStream_Sonic/4x-16        1.49ms ± 2%    0.59ms ± 7%  -59.98%  (p=0.000 n=10+10)
DecodeStream_Sonic/double-16     679µs ± 3%     237µs ± 5%  -65.00%  (p=0.000 n=10+9)
DecodeStream_Sonic/small-16     27.0µs ± 8%     1.6µs ± 3%  -94.17%  (p=0.000 n=10+9)

name                          old alloc/op   new alloc/op   delta
DecodeStream_Sonic/halt-16       265kB ± 0%     142kB ± 1%  -46.38%  (p=0.000 n=10+10)
DecodeStream_Sonic/double-16    1.20MB ± 0%    0.28MB ± 0%  -76.65%  (p=0.000 n=10+10)
DecodeStream_Sonic/4x-16        2.79MB ± 0%    0.56MB ± 0%  -79.87%  (p=0.000 n=10+10)
DecodeStream_Sonic/single-16    1.32MB ± 0%    0.14MB ± 0%  -89.36%  (p=0.000 n=10+9)
DecodeStream_Sonic/small-16      269kB ± 0%       1kB ± 0%  -99.45%  (p=0.000 n=10+10)

name                          old allocs/op  new allocs/op  delta
DecodeStream_Sonic/small-16       21.0 ± 0%      20.0 ± 0%   -4.76%  (p=0.000 n=10+10)
DecodeStream_Sonic/halt-16        12.0 ± 0%       9.0 ± 0%  -25.00%  (p=0.000 n=10+10)
DecodeStream_Sonic/double-16      18.0 ± 0%       9.0 ± 0%  -50.00%  (p=0.000 n=10+10)
DecodeStream_Sonic/4x-16          29.0 ± 0%      14.0 ± 0%  -51.72%  (p=0.000 n=10+10)
DecodeStream_Sonic/single-16      11.0 ± 0%       5.0 ± 0%  -54.55%  (p=0.000 n=10+10)

@AsterDY AsterDY marked this pull request as draft November 6, 2023 12:49
@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2023

Codecov Report

Merging #550 (eb1f9e4) into main (fd7e221) will increase coverage by 0.28%.
Report is 3 commits behind head on main.
The diff coverage is 97.18%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #550      +/-   ##
==========================================
+ Coverage   78.30%   78.59%   +0.28%     
==========================================
  Files          69       69              
  Lines       10686    10692       +6     
==========================================
+ Hits         8368     8403      +35     
+ Misses       1955     1924      -31     
- Partials      363      365       +2     
Files Coverage Δ
internal/decoder/stream.go 92.17% <97.18%> (+28.87%) ⬆️

... and 1 file with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@AsterDY
Copy link
Collaborator Author

AsterDY commented Nov 7, 2023

fix #378

@AsterDY AsterDY marked this pull request as ready for review November 7, 2023 03:26
@AsterDY AsterDY merged commit b84e227 into main Nov 7, 2023
30 checks passed
@AsterDY AsterDY deleted the opt/dec_buf branch November 7, 2023 06:24
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.

4 participants