V3 Backport : Reduce the number of memory allocations in lossless WebP encoder #2947
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR backports performance improvements for the lossless WebP encoder by reducing memory allocations and optimizing backward reference handling. Key changes include:
- Updating test and production code to use the new MemoryAllocator-based constructors for Vp8LBackwardRefs.
- Converting PixOrCopy from a class to a readonly struct and updating its creation methods.
- Refactoring loops to use foreach and in/ref semantics, as well as streamlining histogram and backward reference operations.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs | Updated instantiation of Vp8LBackwardRefs and literal token creation. |
| src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs | Changed iteration in StoreRefs and updated AddSinglePixOrCopy to use in parameters. |
| src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs | Removed block size calculation and updated references disposal. |
| src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs | Migrated from List to MemoryAllocator-based storage and added a Clear method. |
| src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs | Converted to a readonly struct with updated factory methods for immutability. |
| src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs | Refactored iteration logic and updated HistoListUpdateHead to require an index parameter. |
| src/ImageSharp/Formats/Webp/Lossless/CostModel.cs | Replaced loop constructs with foreach for direct reference iteration. |
| src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs | Updated loops to use ref iteration and replaced clear methods for consistency. |
Comments suppressed due to low confidence (2)
src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs:782
- [nitpick] Ensure that using ref iteration to mutate elements of a readonly struct is intended and safe in this context.
foreach (ref PixOrCopy v in refs)
src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs:467
- [nitpick] Verify that passing the index 'j' to HistoListUpdateHead correctly maintains the intended order of histogram pairs.
HistoListUpdateHead(histoPriorityList, p, j);
| this.count = 0; | ||
| } | ||
|
|
||
| public void Add(PixOrCopy pixOrCopy) => this.refs.Memory.Span[this.count++] = pixOrCopy; |
There was a problem hiding this comment.
Consider adding a boundary check to ensure 'this.count' does not exceed the allocated memory length to prevent potential overflows.
Suggested change
| public void Add(PixOrCopy pixOrCopy) => this.refs.Memory.Span[this.count++] = pixOrCopy; | |
| public void Add(PixOrCopy pixOrCopy) | |
| { | |
| if (this.count >= this.refs.Memory.Length) | |
| { | |
| throw new InvalidOperationException("Cannot add more elements. Memory limit exceeded."); | |
| } | |
| this.refs.Memory.Span[this.count++] = pixOrCopy; | |
| } |
Copilot uses AI. Check for mistakes.
This was referenced Jul 23, 2025
This was referenced Jul 30, 2025
This was referenced Jul 30, 2025
This was referenced Aug 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisites
Description
Backport of #2940 for V3.