From 021ac8b15f391ace474c83f600ab40a71ac5c6b1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 16 Jun 2021 01:15:15 +1000 Subject: [PATCH] Fix buffer allocation --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 2 +- .../Allocators/ArrayPoolMemoryAllocator.cs | 4 ---- .../Quantization/EuclideanPixelMap{TPixel}.cs | 23 +++++++------------ .../ArrayPoolMemoryAllocatorTests.cs | 13 ----------- 4 files changed, 9 insertions(+), 33 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index c03104779e..585f87b3e8 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -150,7 +150,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // The palette quantizer can reuse the same pixel map across multiple frames // since the palette is unchanging. This allows a reduction of memory usage across // multi frame gifs using a global palette. - Unsafe.SkipInit(out EuclideanPixelMap pixelMap); + EuclideanPixelMap pixelMap = default; bool pixelMapHasValue = false; for (int i = 0; i < image.Frames.Count; i++) { diff --git a/src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.cs index 4a3c42910f..a79e042a32 100644 --- a/src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.cs @@ -131,10 +131,6 @@ namespace SixLabors.ImageSharp.Memory Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); int itemSizeBytes = Unsafe.SizeOf(); int bufferSizeInBytes = length * itemSizeBytes; - if (bufferSizeInBytes < 0 || bufferSizeInBytes > this.BufferCapacityInBytes) - { - ThrowInvalidAllocationException(length, this.BufferCapacityInBytes); - } ArrayPool pool = this.GetArrayPool(bufferSizeInBytes); byte[] byteArray = pool.Rent(bufferSizeInBytes); diff --git a/src/ImageSharp/Processing/Processors/Quantization/EuclideanPixelMap{TPixel}.cs b/src/ImageSharp/Processing/Processors/Quantization/EuclideanPixelMap{TPixel}.cs index 9b626303bd..0311c40be4 100644 --- a/src/ImageSharp/Processing/Processors/Quantization/EuclideanPixelMap{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Quantization/EuclideanPixelMap{TPixel}.cs @@ -5,6 +5,7 @@ using System; using System.Buffers; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Processing.Processors.Quantization @@ -33,7 +34,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization { this.Palette = palette; this.rgbaPalette = new Rgba32[palette.Length]; - this.cache = ColorDistanceCache.Create(); + this.cache = new ColorDistanceCache(configuration.MemoryAllocator); PixelOperations.Instance.ToRgba32(configuration, this.Palette.Span, this.rgbaPalette); } @@ -141,26 +142,18 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization private const int RgbShift = 8 - IndexBits; private const int AlphaShift = 8 - IndexAlphaBits; private const int Entries = IndexCount * IndexCount * IndexCount * IndexAlphaCount; - private const int BufferLength = (Entries + 1) >> 1; private MemoryHandle tableHandle; - private readonly int[] table; + private readonly IMemoryOwner table; private readonly short* tablePointer; - private ColorDistanceCache(int bufferLength, int entries) + public ColorDistanceCache(MemoryAllocator allocator) { - // We use ArrayPool.Shared for several reasons. - // 1. To avoid out of range issues caused by configuring small discontiguous buffers rented via MemoryAllocator - // 2. To ensure that the rented buffer is actually pooled. - // 3. The .NET runtime already uses this pool so we might already have a pooled array present. - this.table = ArrayPool.Shared.Rent(bufferLength); - this.tableHandle = this.table.AsMemory().Pin(); - new Span(this.tableHandle.Pointer, entries).Fill(-1); + this.table = allocator.Allocate(Entries); + this.table.GetSpan().Fill(-1); + this.tableHandle = this.table.Memory.Pin(); this.tablePointer = (short*)this.tableHandle.Pointer; } - public static ColorDistanceCache Create() - => new ColorDistanceCache(BufferLength, Entries); - [MethodImpl(InliningOptions.ShortMethod)] public void Add(Rgba32 rgba, byte index) { @@ -199,8 +192,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization { if (this.table != null) { - ArrayPool.Shared.Return(this.table); this.tableHandle.Dispose(); + this.table.Dispose(); } } } diff --git a/tests/ImageSharp.Tests/Memory/Allocators/ArrayPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/ArrayPoolMemoryAllocatorTests.cs index 939e5898cd..50ec09ce3f 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/ArrayPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/ArrayPoolMemoryAllocatorTests.cs @@ -223,19 +223,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators Assert.Equal(0, buffer.Memory.Length); } - [Theory] - [InlineData(101)] - [InlineData((int.MaxValue / SizeOfLargeStruct) - 1)] - [InlineData(int.MaxValue / SizeOfLargeStruct)] - [InlineData((int.MaxValue / SizeOfLargeStruct) + 1)] - [InlineData((int.MaxValue / SizeOfLargeStruct) + 137)] - public void Allocate_OverCapacity_Throws_InvalidMemoryOperationException(int length) - { - this.LocalFixture.MemoryAllocator.BufferCapacityInBytes = 100 * SizeOfLargeStruct; - Assert.Throws(() => - this.LocalFixture.MemoryAllocator.Allocate(length)); - } - [Theory] [InlineData(-1)] public void AllocateManagedByteBuffer_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length)