-
-
Notifications
You must be signed in to change notification settings - Fork 878
Reduce the number of memory allocations in lossless WebP encoder #2940
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
Conversation
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.
Just a quick read so far but looking good. I'll have to pull it down to review properly.
{ | ||
PixOrCopy v = backwardRefsEnumerator.Current; | ||
int ix = ((y >> histoBits) * histoXSize) + (x >> histoBits); | ||
histograms[ix].AddSinglePixOrCopy(v, false); |
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.
This method could be updated to take the struct via in
to avoid the copy.
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.
I've made the change. But if there is any performance difference, it's hard to notice. The structure is not that big.
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.
Looks good after skimming through all changes.
I'm thinking of backporting this to V3. @antonfirsov what do you think? |
The change looks simple and safe enough to backport. |
Thanks for this @SladeThe it's very much appreciated!! |
This PR closes #2934
The results of benchmark
#55 EncodeWebp
:Before
After
Pin the refs (without the capacity guard)