Merge pull request #2940 from SladeThe/main

Reduce the number of memory allocations in lossless WebP encoder

src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs

@@ -149,9 +149,8 @@ internal static class BackwardReferenceEncoder
         }
 
         // Find the cacheBits giving the lowest entropy.
-        for (int idx = 0; idx < refs.Refs.Count; idx++)
+        foreach (PixOrCopy v in refs)
         {
-            PixOrCopy v = refs.Refs[idx];
             if (v.IsLiteral())
             {
                 uint pix = bgra[pos++];
@@ -387,7 +386,7 @@ internal static class BackwardReferenceEncoder
             colorCache = new ColorCache(cacheBits);
         }
 
-        backwardRefs.Refs.Clear();
+        backwardRefs.Clear();
         for (int ix = 0; ix < chosenPathSize; ix++)
         {
             int len = chosenPath[ix];
@@ -479,7 +478,7 @@ internal static class BackwardReferenceEncoder
             colorCache = new ColorCache(cacheBits);
         }
 
-        refs.Refs.Clear();
+        refs.Clear();
         for (int i = 0; i < pixCount;)
         {
             // Alternative #1: Code the pixels starting at 'i' using backward reference.
@@ -734,7 +733,7 @@ internal static class BackwardReferenceEncoder
             colorCache = new ColorCache(cacheBits);
         }
 
-        refs.Refs.Clear();
+        refs.Clear();
 
         // Add first pixel as literal.
         AddSingleLiteral(bgra[0], useColorCache, colorCache, refs);
@@ -779,10 +778,9 @@ internal static class BackwardReferenceEncoder
     private static void BackwardRefsWithLocalCache(ReadOnlySpan<uint> bgra, int cacheBits, Vp8LBackwardRefs refs)
     {
         int pixelIndex = 0;
-        ColorCache colorCache = new ColorCache(cacheBits);
+        ColorCache colorCache = new(cacheBits);
-        for (int idx = 0; idx < refs.Refs.Count; idx++)
+        foreach (ref PixOrCopy v in refs)
         {
-            PixOrCopy v = refs.Refs[idx];
             if (v.IsLiteral())
             {
                 uint bgraLiteral = v.BgraOrDistance;
@@ -790,9 +788,7 @@ internal static class BackwardReferenceEncoder
                 if (ix >= 0)
                 {
                     // Color cache contains bgraLiteral
-                    v.Mode = PixOrCopyMode.CacheIdx;
+                    v = PixOrCopy.CreateCacheIdx(ix);
-                    v.BgraOrDistance = (uint)ix;
-                    v.Len = 1;
                 }
                 else
                 {
@@ -814,14 +810,13 @@ internal static class BackwardReferenceEncoder
 
     private static void BackwardReferences2DLocality(int xSize, Vp8LBackwardRefs refs)
     {
-        using List<PixOrCopy>.Enumerator c = refs.Refs.GetEnumerator();
+        foreach (ref PixOrCopy v in refs)
-        while (c.MoveNext())
         {
-            if (c.Current.IsCopy())
+            if (v.IsCopy())
             {
-                int dist = (int)c.Current.BgraOrDistance;
+                int dist = (int)v.BgraOrDistance;
                 int transformedDist = DistanceToPlaneCode(xSize, dist);
-                c.Current.BgraOrDistance = (uint)transformedDist;
+                v = PixOrCopy.CreateCopy((uint)transformedDist, v.Len);
             }
         }
     }

src/ImageSharp/Formats/Webp/Lossless/CostModel.cs

@@ -40,9 +40,9 @@ internal class CostModel
         using OwnedVp8LHistogram histogram = OwnedVp8LHistogram.Create(this.memoryAllocator, cacheBits);
 
         // The following code is similar to HistogramCreate but converts the distance to plane code.
-        for (int i = 0; i < backwardRefs.Refs.Count; i++)
+        foreach (PixOrCopy v in backwardRefs)
         {
-            histogram.AddSinglePixOrCopy(backwardRefs.Refs[i], true, xSize);
+            histogram.AddSinglePixOrCopy(in v, true, xSize);
         }
 
         ConvertPopulationCountTableToBitEstimates(histogram.NumCodes(), histogram.Literal, this.Literal);

src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs

@@ -109,12 +109,11 @@ internal static class HistogramEncoder
     {
         int x = 0, y = 0;
         int histoXSize = LosslessUtils.SubSampleSize(xSize, histoBits);
-        using List<PixOrCopy>.Enumerator backwardRefsEnumerator = backwardRefs.Refs.GetEnumerator();
+
-        while (backwardRefsEnumerator.MoveNext())
+        foreach (PixOrCopy v in backwardRefs)
         {
-            PixOrCopy v = backwardRefsEnumerator.Current;
             int ix = ((y >> histoBits) * histoXSize) + (x >> histoBits);
-            histograms[ix].AddSinglePixOrCopy(v, false);
+            histograms[ix].AddSinglePixOrCopy(in v, false);
             x += v.Len;
             while (x >= xSize)
             {
@@ -217,7 +216,7 @@ internal static class HistogramEncoder
             clusterMappings[idx] = (ushort)idx;
         }
 
-        List<int> indicesToRemove = new();
+        List<int> indicesToRemove = [];
         Vp8LStreaks stats = new();
         Vp8LBitEntropy bitsEntropy = new();
         for (int idx = 0; idx < histograms.Count; idx++)
@@ -345,7 +344,7 @@ internal static class HistogramEncoder
 
         // Priority list of histogram pairs. Its size impacts the quality of the compression and the speed:
         // the smaller the faster but the worse for the compression.
-        List<HistogramPair> histoPriorityList = new();
+        List<HistogramPair> histoPriorityList = [];
         const int maxSize = 9;
 
         // Fill the initial mapping.
@@ -465,7 +464,7 @@ internal static class HistogramEncoder
                     }
                 }
 
-                HistoListUpdateHead(histoPriorityList, p);
+                HistoListUpdateHead(histoPriorityList, p, j);
                 j++;
             }
 
@@ -480,7 +479,7 @@ internal static class HistogramEncoder
         int histoSize = histograms.Count(h => h != null);
 
         // Priority list of histogram pairs.
-        List<HistogramPair> histoPriorityList = new();
+        List<HistogramPair> histoPriorityList = [];
         int maxSize = histoSize * histoSize;
         Vp8LStreaks stats = new();
         Vp8LBitEntropy bitsEntropy = new();
@@ -525,7 +524,7 @@ internal static class HistogramEncoder
                 }
                 else
                 {
-                    HistoListUpdateHead(histoPriorityList, p);
+                    HistoListUpdateHead(histoPriorityList, p, i);
                     i++;
                 }
             }
@@ -647,7 +646,7 @@ internal static class HistogramEncoder
 
         histoList.Add(pair);
 
-        HistoListUpdateHead(histoList, pair);
+        HistoListUpdateHead(histoList, pair, histoList.Count - 1);
 
         return pair.CostDiff;
     }
@@ -674,13 +673,11 @@ internal static class HistogramEncoder
     /// <summary>
     /// Check whether a pair in the list should be updated as head or not.
     /// </summary>
-    private static void HistoListUpdateHead(List<HistogramPair> histoList, HistogramPair pair)
+    private static void HistoListUpdateHead(List<HistogramPair> histoList, HistogramPair pair, int idx)
     {
         if (pair.CostDiff < histoList[0].CostDiff)
         {
-            // Replace the best pair.
+            histoList[idx] = histoList[0];
-            int oldIdx = histoList.IndexOf(pair);
-            histoList[oldIdx] = histoList[0];
             histoList[0] = pair;
         }
     }

src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs

@@ -6,37 +6,24 @@ using System.Diagnostics;
 namespace SixLabors.ImageSharp.Formats.Webp.Lossless;
 
 [DebuggerDisplay("Mode: {Mode}, Len: {Len}, BgraOrDistance: {BgraOrDistance}")]
-internal sealed class PixOrCopy
+internal readonly struct PixOrCopy
 {
-    public PixOrCopyMode Mode { get; set; }
+    public readonly PixOrCopyMode Mode;
-
+    public readonly ushort Len;
-    public ushort Len { get; set; }
+    public readonly uint BgraOrDistance;
-
+
-    public uint BgraOrDistance { get; set; }
+    private PixOrCopy(PixOrCopyMode mode, ushort len, uint bgraOrDistance)
-
-    public static PixOrCopy CreateCacheIdx(int idx) =>
-        new PixOrCopy
-        {
-            Mode = PixOrCopyMode.CacheIdx,
-            BgraOrDistance = (uint)idx,
-            Len = 1
-        };
-
-    public static PixOrCopy CreateLiteral(uint bgra) =>
-        new PixOrCopy
-        {
-            Mode = PixOrCopyMode.Literal,
-            BgraOrDistance = bgra,
-            Len = 1
-        };
-
-    public static PixOrCopy CreateCopy(uint distance, ushort len) =>
-        new PixOrCopy
     {
-        Mode = PixOrCopyMode.Copy,
+        this.Mode = mode;
-        BgraOrDistance = distance,
+        this.Len = len;
-        Len = len
+        this.BgraOrDistance = bgraOrDistance;
-    };
+    }
+
+    public static PixOrCopy CreateCacheIdx(int idx) => new(PixOrCopyMode.CacheIdx, 1, (uint)idx);
+
+    public static PixOrCopy CreateLiteral(uint bgra) => new(PixOrCopyMode.Literal, 1, bgra);
+
+    public static PixOrCopy CreateCopy(uint distance, ushort len) => new(PixOrCopyMode.Copy, len, distance);
 
     public int Literal(int component) => (int)(this.BgraOrDistance >> (component * 8)) & 0xFF;
 

src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs

@@ -1,21 +1,28 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
+using System.Buffers;
+using SixLabors.ImageSharp.Memory;
+
 namespace SixLabors.ImageSharp.Formats.Webp.Lossless;
 
-internal class Vp8LBackwardRefs
+internal class Vp8LBackwardRefs : IDisposable
 {
-    public Vp8LBackwardRefs(int pixels) => this.Refs = new List<PixOrCopy>(pixels);
+    private readonly IMemoryOwner<PixOrCopy> refs;
+    private int count;
+
+    public Vp8LBackwardRefs(MemoryAllocator memoryAllocator, int pixels)
+    {
+        this.refs = memoryAllocator.Allocate<PixOrCopy>(pixels);
+        this.count = 0;
+    }
+
+    public void Add(PixOrCopy pixOrCopy) => this.refs.Memory.Span[this.count++] = pixOrCopy;
 
-    /// <summary>
+    public void Clear() => this.count = 0;
-    /// Gets or sets the common block-size.
-    /// </summary>
-    public int BlockSize { get; set; }
 
-    /// <summary>
+    public Span<PixOrCopy>.Enumerator GetEnumerator() => this.refs.Slice(0, this.count).GetEnumerator();
-    /// Gets the backward references.
-    /// </summary>
-    public List<PixOrCopy> Refs { get; }
 
-    public void Add(PixOrCopy pixOrCopy) => this.Refs.Add(pixOrCopy);
+    /// <inheritdoc/>
+    public void Dispose() => this.refs.Dispose();
 }

src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs

@@ -26,9 +26,9 @@ internal class Vp8LEncoder : IDisposable
     /// </summary>
     private ScratchBuffer scratch;  // mutable struct, don't make readonly
 
-    private readonly int[][] histoArgb = { new int[256], new int[256], new int[256], new int[256] };
+    private readonly int[][] histoArgb = [new int[256], new int[256], new int[256], new int[256]];
 
-    private readonly int[][] bestHisto = { new int[256], new int[256], new int[256], new int[256] };
+    private readonly int[][] bestHisto = [new int[256], new int[256], new int[256], new int[256]];
 
     /// <summary>
     /// The <see cref="MemoryAllocator"/> to use for buffer allocations.
@@ -45,11 +45,6 @@ internal class Vp8LEncoder : IDisposable
     /// </summary>
     private const int MaxRefsBlockPerImage = 16;
 
-    /// <summary>
-    /// Minimum block size for backward references.
-    /// </summary>
-    private const int MinBlockSize = 256;
-
     /// <summary>
     /// A bit writer for writing lossless webp streams.
     /// </summary>
@@ -136,14 +131,9 @@ internal class Vp8LEncoder : IDisposable
         this.Refs = new Vp8LBackwardRefs[3];
         this.HashChain = new Vp8LHashChain(memoryAllocator, pixelCount);
 
-        // We round the block size up, so we're guaranteed to have at most MaxRefsBlockPerImage blocks used:
-        int refsBlockSize = ((pixelCount - 1) / MaxRefsBlockPerImage) + 1;
         for (int i = 0; i < this.Refs.Length; i++)
         {
-            this.Refs[i] = new Vp8LBackwardRefs(pixelCount)
+            this.Refs[i] = new Vp8LBackwardRefs(memoryAllocator, pixelCount);
-            {
-                BlockSize = refsBlockSize < MinBlockSize ? MinBlockSize : refsBlockSize
-            };
         }
     }
 
@@ -151,10 +141,10 @@ internal class Vp8LEncoder : IDisposable
     // This sequence is tuned from that, but more weighted for lower symbol count,
     // and more spiking histograms.
     // This uses C#'s compiler optimization to refer to assembly's static data directly.
-    private static ReadOnlySpan<byte> StorageOrder => new byte[] { 17, 18, 0, 1, 2, 3, 4, 5, 16, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 };
+    private static ReadOnlySpan<byte> StorageOrder => [17, 18, 0, 1, 2, 3, 4, 5, 16, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
 
     // This uses C#'s compiler optimization to refer to assembly's static data directly.
-    private static ReadOnlySpan<byte> Order => new byte[] { 1, 2, 0, 3 };
+    private static ReadOnlySpan<byte> Order => [1, 2, 0, 3];
 
     /// <summary>
     /// Gets the memory for the image data as packed bgra values.
@@ -547,7 +537,7 @@ internal class Vp8LEncoder : IDisposable
         EntropyIx entropyIdx = this.AnalyzeEntropy(bgra, width, height, usePalette, this.PaletteSize, this.TransformBits, out redAndBlueAlwaysZero);
 
         bool doNotCache = false;
-        List<CrunchConfig> crunchConfigs = new();
+        List<CrunchConfig> crunchConfigs = [];
 
         if (this.method == WebpEncodingMethod.BestQuality && this.quality == 100)
         {
@@ -593,7 +583,7 @@ internal class Vp8LEncoder : IDisposable
             }
         }
 
-        return crunchConfigs.ToArray();
+        return [.. crunchConfigs];
     }
 
     private void EncodeImage(int width, int height, bool useCache, CrunchConfig config, int cacheBits, bool lowEffort)
@@ -1068,9 +1058,8 @@ internal class Vp8LEncoder : IDisposable
         int histogramIx = histogramSymbols[0];
         Span<HuffmanTreeCode> codes = huffmanCodes.AsSpan(5 * histogramIx);
 
-        for (int i = 0; i < backwardRefs.Refs.Count; i++)
+        foreach (PixOrCopy v in backwardRefs)
         {
-            PixOrCopy v = backwardRefs.Refs[i];
             if (tileX != (x & tileMask) || tileY != (y & tileMask))
             {
                 tileX = x & tileMask;
@@ -1265,13 +1254,13 @@ internal class Vp8LEncoder : IDisposable
         // non-zero red and blue values. If all are zero, we can later skip
         // the cross color optimization.
         byte[][] histoPairs =
-        {
+        [
-            new[] { (byte)HistoIx.HistoRed, (byte)HistoIx.HistoBlue },
+            [(byte)HistoIx.HistoRed, (byte)HistoIx.HistoBlue],
-            new[] { (byte)HistoIx.HistoRedPred, (byte)HistoIx.HistoBluePred },
+            [(byte)HistoIx.HistoRedPred, (byte)HistoIx.HistoBluePred],
-            new[] { (byte)HistoIx.HistoRedSubGreen, (byte)HistoIx.HistoBlueSubGreen },
+            [(byte)HistoIx.HistoRedSubGreen, (byte)HistoIx.HistoBlueSubGreen],
-            new[] { (byte)HistoIx.HistoRedPredSubGreen, (byte)HistoIx.HistoBluePredSubGreen },
+            [(byte)HistoIx.HistoRedPredSubGreen, (byte)HistoIx.HistoBluePredSubGreen],
-            new[] { (byte)HistoIx.HistoRed, (byte)HistoIx.HistoBlue }
+            [(byte)HistoIx.HistoRed, (byte)HistoIx.HistoBlue]
-        };
+        ];
         Span<uint> redHisto = histo[(256 * histoPairs[(int)minEntropyIx][0])..];
         Span<uint> blueHisto = histo[(256 * histoPairs[(int)minEntropyIx][1])..];
         for (int i = 1; i < 256; i++)
@@ -1325,7 +1314,7 @@ internal class Vp8LEncoder : IDisposable
     /// <returns>The number of palette entries.</returns>
     private static int GetColorPalette(ReadOnlySpan<uint> bgra, int width, int height, Span<uint> palette)
     {
-        HashSet<uint> colors = new();
+        HashSet<uint> colors = [];
         for (int y = 0; y < height; y++)
         {
             ReadOnlySpan<uint> bgraRow = bgra.Slice(y * width, width);
@@ -1904,9 +1893,9 @@ internal class Vp8LEncoder : IDisposable
     /// </summary>
     public void ClearRefs()
     {
-        foreach (Vp8LBackwardRefs t in this.Refs)
+        foreach (Vp8LBackwardRefs refs in this.Refs)
         {
-            t.Refs.Clear();
+            refs.Clear();
         }
     }
 
@@ -1918,6 +1907,12 @@ internal class Vp8LEncoder : IDisposable
         this.BgraScratch?.Dispose();
         this.Palette.Dispose();
         this.TransformData?.Dispose();
+
+        foreach (Vp8LBackwardRefs refs in this.Refs)
+        {
+            refs.Dispose();
+        }
+
         this.HashChain.Dispose();
     }
 

src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs

@@ -138,9 +138,9 @@ internal abstract unsafe class Vp8LHistogram
     /// <param name="refs">The backward references.</param>
     public void StoreRefs(Vp8LBackwardRefs refs)
     {
-        for (int i = 0; i < refs.Refs.Count; i++)
+        foreach (PixOrCopy v in refs)
         {
-            this.AddSinglePixOrCopy(refs.Refs[i], false);
+            this.AddSinglePixOrCopy(in v, false);
         }
     }
 
@@ -150,7 +150,7 @@ internal abstract unsafe class Vp8LHistogram
     /// <param name="v">The token to add.</param>
     /// <param name="useDistanceModifier">Indicates whether to use the distance modifier.</param>
     /// <param name="xSize">xSize is only used when useDistanceModifier is true.</param>
-    public void AddSinglePixOrCopy(PixOrCopy v, bool useDistanceModifier, int xSize = 0)
+    public void AddSinglePixOrCopy(in PixOrCopy v, bool useDistanceModifier, int xSize = 0)
     {
         if (v.IsLiteral())
         {

tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs

@@ -66,18 +66,14 @@ public class Vp8LHistogramTests
         // All remaining values are expected to be zero.
         literals.AsSpan().CopyTo(expectedLiterals);
 
-        Vp8LBackwardRefs backwardRefs = new(pixelData.Length);
+        MemoryAllocator memoryAllocator = Configuration.Default.MemoryAllocator;
+
+        using Vp8LBackwardRefs backwardRefs = new(memoryAllocator, pixelData.Length);
         for (int i = 0; i < pixelData.Length; i++)
         {
-            backwardRefs.Add(new PixOrCopy()
+            backwardRefs.Add(PixOrCopy.CreateLiteral(pixelData[i]));
-            {
-                BgraOrDistance = pixelData[i],
-                Len = 1,
-                Mode = PixOrCopyMode.Literal
-            });
         }
 
-        MemoryAllocator memoryAllocator = Configuration.Default.MemoryAllocator;
         using OwnedVp8LHistogram histogram0 = OwnedVp8LHistogram.Create(memoryAllocator, backwardRefs, 3);
         using OwnedVp8LHistogram histogram1 = OwnedVp8LHistogram.Create(memoryAllocator, backwardRefs, 3);
         for (int i = 0; i < 5; i++)