Skip to content

Reduce the number of memory allocations in lossless WebP encoder - #2940

Merged
JimBobSquarePants merged 8 commits into
SixLabors:mainfrom
SladeThe:main
Jun 13, 2025
Merged

Reduce the number of memory allocations in lossless WebP encoder#2940
JimBobSquarePants merged 8 commits into
SixLabors:mainfrom
SladeThe:main

Conversation

@SladeThe

@SladeTheSladeThe commented Jun 6, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR closes#2934

The results of benchmark #55 EncodeWebp:

Before

MethodMeanErrorStdDevGen0Gen1Gen2Allocated
'ImageSharp Webp Lossless'105.31 ms24.582 ms1.347 ms2000.00001800.00001400.000020490.55 KB

After

MethodMeanErrorStdDevGen0Gen1Gen2Allocated
'ImageSharp Webp Lossless'100.61 ms15.196 ms0.833 ms2000.00002000.00002000.00003904.68 KB

Pin the refs (without the capacity guard)

MethodMeanErrorStdDevGen0Gen1Gen2Allocated
'ImageSharp Webp Lossless'97.43 ms9.375 ms0.514 ms2000.00002000.00002000.00003904.98 KB

@CLAassistant

CLAassistant commented Jun 6, 2025•
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

JimBobSquarePants
JimBobSquarePants reviewed Jun 9, 2025
View reviewed changes

@JimBobSquarePantsJimBobSquarePants left a comment

Copy link
Copy Markdown
Member

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.

Comment threadsrc/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs Outdated
{
PixOrCopy v = backwardRefsEnumerator.Current;
int ix = ((y >> histoBits) * histoXSize) + (x >> histoBits);
histograms[ix].AddSinglePixOrCopy(v, false);

@JimBobSquarePantsJimBobSquarePantsJun 9, 2025

Copy link
Copy Markdown
Member

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.

@SladeTheSladeTheJun 10, 2025

Copy link
Copy Markdown
ContributorAuthor

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.

Comment threadsrc/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs
antonfirsov
antonfirsov approved these changes Jun 11, 2025
View reviewed changes

@antonfirsovantonfirsov left a comment

Copy link
Copy Markdown
Member

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.

@JimBobSquarePants

JimBobSquarePants commented Jun 12, 2025

Copy link
Copy Markdown
Member

I'm thinking of backporting this to V3. @antonfirsov what do you think?

@antonfirsov

antonfirsov commented Jun 12, 2025

Copy link
Copy Markdown
Member

The change looks simple and safe enough to backport.

@JimBobSquarePants
JimBobSquarePants merged commit 1e58ba9 into SixLabors:mainJun 13, 2025
@JimBobSquarePants

JimBobSquarePants commented Jun 13, 2025

Copy link
Copy Markdown
Member

Thanks for this @SladeThe it's very much appreciated!!

JimBobSquarePants added a commit that referenced this pull request Jun 13, 2025
@JimBobSquarePants
Backport#2940
57f58fe
@JimBobSquarePantsJimBobSquarePants mentioned this pull request Jun 13, 2025
Merged
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JimBobSquarePantsJimBobSquarePantsJimBobSquarePants left review comments
@antonfirsovantonfirsovantonfirsov approved these changes

Assignees

No one assigned

Labels

area:performanceformats:webp

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lossless WebP encoder allocates A LOT

4 participants

@SladeThe@CLAassistant@JimBobSquarePants@antonfirsov