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

refactoring: replace a byte.Buffer with a byte slice #7

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

hajimehoshi
Copy link
Contributor

This change also removes length member with the len of the slice.

This slightly make the audio decoding performance worse, but I am not sure why.

goos: darwin
goarch: arm64
pkg: github.com/gen2brain/mpeg
               │   old.txt   │              new.txt               │
               │   sec/op    │   sec/op     vs base               │
DecodeVideo-12   76.90µ ± 1%   76.34µ ± 1%  -0.73% (p=0.024 n=10)
DecodeAudio-12   47.64µ ± 1%   49.36µ ± 2%  +3.62% (p=0.002 n=10)
RGBA-12          41.29µ ± 0%   40.53µ ± 0%  -1.83% (p=0.001 n=10)
geomean          53.28µ        53.45µ       +0.32%

               │  old.txt   │              new.txt              │
               │    B/op    │    B/op     vs base               │
DecodeVideo-12   76.00 ± 1%   75.00 ± 0%  -1.32% (p=0.008 n=10)
DecodeAudio-12   46.50 ± 1%   48.00 ± 2%  +3.23% (p=0.000 n=10)
RGBA-12          40.00 ± 2%   40.00 ± 0%       ~ (p=0.582 n=10)
geomean          52.09        52.41       +0.62%

               │   old.txt    │               new.txt               │
               │  allocs/op   │ allocs/op   vs base                 │
DecodeVideo-12   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
DecodeAudio-12   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
RGBA-12          0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                     ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

This change also removes `length` member with the len of the slice.
buffer.go Show resolved Hide resolved
@gen2brain
Copy link
Owner

I remember I switched back and forth between byte slice and bytes.Buffer, I can't remember what was the difference and why bytes.Buffer is finally used.

@hajimehoshi
Copy link
Contributor Author

From my intuition, the current byte.Buffer just adds overheads, but I might be wrong.

@gen2brain gen2brain merged commit a2ac4fc into gen2brain:main Apr 12, 2024
1 check passed
@gen2brain
Copy link
Owner

Merged, thanks.

@hajimehoshi hajimehoshi deleted the bytes branch April 12, 2024 15:45
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