PixelDataPool<T>: reduce maximum pooled array size to 32MB for all value types

src/ImageSharp/Memory/PixelDataPool{T}.cs

@@ -2,7 +2,7 @@
 // Licensed under the Apache License, Version 2.0.
 
 using System.Buffers;
-using SixLabors.ImageSharp.PixelFormats;
+using System.Runtime.CompilerServices;
 
 namespace SixLabors.ImageSharp.Memory
 {
@@ -13,10 +13,21 @@ namespace SixLabors.ImageSharp.Memory
     internal class PixelDataPool<T>
         where T : struct
     {
+        /// <summary>
+        /// The maximum size of pooled arrays in bytes.
+        /// Currently set to 32MB, which is equivalent to 8 megapixels of raw <see cref="Rgba32"/> data.
+        /// </summary>
+        private const int MaxPooledBufferSizeInBytes = 32 * 1024 * 1024;
+
+        /// <summary>
+        /// The maximum array length of the <see cref="ArrayPool"/>.
+        /// </summary>
+        private static readonly int MaxArrayLength = MaxPooledBufferSizeInBytes / Unsafe.SizeOf<T>();
+
         /// <summary>
         /// The <see cref="ArrayPool{T}"/> which is not kept clean.
         /// </summary>
-        private static readonly ArrayPool<T> ArrayPool = ArrayPool<T>.Create(CalculateMaxArrayLength(), 50);
+        private static readonly ArrayPool<T> ArrayPool = ArrayPool<T>.Create(MaxArrayLength, 50);
 
         /// <summary>
         /// Rents the pixel array from the pool.
@@ -36,24 +47,5 @@ namespace SixLabors.ImageSharp.Memory
         {
             ArrayPool.Return(array);
         }
-
-        /// <summary>
-        /// Heuristically calculates a reasonable maxArrayLength value for the backing <see cref="ArrayPool{T}"/>.
-        /// </summary>
-        /// <returns>The maxArrayLength value</returns>
-        internal static int CalculateMaxArrayLength()
-        {
-            // ReSharper disable once SuspiciousTypeConversion.Global
-            if (default(T) is IPixel)
-            {
-                const int MaximumExpectedImageSize = 16384 * 16384;
-                return MaximumExpectedImageSize;
-            }
-            else
-            {
-                const int MaxArrayLength = 1024 * 1024; // Match default pool.
-                return MaxArrayLength;
-            }
-        }
     }
 }

tests/ImageSharp.Tests/Memory/PixelDataPoolTests.cs

@@ -1,20 +1,21 @@
 // Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
-
+using System.Linq;
+using System.Runtime.InteropServices;
+using SixLabors.ImageSharp.Memory;
+using Xunit;
 
 // ReSharper disable InconsistentNaming
 namespace SixLabors.ImageSharp.Tests.Memory
 {
-    using SixLabors.ImageSharp.Memory;
-
-    using Xunit;
-
     /// <summary>
     /// Tests the <see cref="PixelDataPool{T}"/> class.
     /// </summary>
     public class PixelDataPoolTests
     {
+        readonly object monitor = new object();
+        
         [Fact]
         public void PixelDataPoolRentsMinimumSize()
         {
@@ -33,23 +34,62 @@ namespace SixLabors.ImageSharp.Tests.Memory
             Assert.True(pixels.Length >= 1024);
         }
 
-        [Theory]
-        [InlineData(false)]
-        [InlineData(true)]
-        public void CalculateMaxArrayLength(bool isRawData)
+        /// <summary>
+        /// Rent 'n' buffers -> return all -> re-rent, verify if there is at least one in common.
+        /// </summary>
+        private bool CheckIsPooled<T>(int n, int count)
+            where T : struct
+        {
+            lock (this.monitor)
+            {
+                T[][] original = new T[n][];
+
+                for (int i = 0; i < n; i++)
+                {
+                    original[i] = PixelDataPool<T>.Rent(count);
+                }
+
+                for (int i = 0; i < n; i++)
+                {
+                    PixelDataPool<T>.Return(original[i]);
+                }
+
+                T[][] verification = new T[n][];
+
+                for (int i = 0; i < n; i++)
+                {
+                    verification[i] = PixelDataPool<T>.Rent(count);
+                }
+
+                return original.Intersect(verification).Any();
+            }
+        }
+
+        [Fact]
+        public void SmallBuffersArePooled()
         {
-            int max = isRawData ? PixelDataPool<int>.CalculateMaxArrayLength()
-                          : PixelDataPool<Rgba32>.CalculateMaxArrayLength();
+            Assert.True(this.CheckIsPooled<byte>(5, 512));
+        }
 
-            Assert.Equal(max > 1024 * 1024, !isRawData);
+        [Fact]
+        public void LargeBuffersAreNotPooled_OfByte()
+        {
+            const int mb128 = 128 * 1024 * 1024;
+            Assert.False(this.CheckIsPooled<byte>(2, mb128));
+        }
+
+        [StructLayout(LayoutKind.Explicit, Size = 512)]
+        struct TestStruct
+        {
         }
 
         [Fact]
-        public void RentNonIPixelData()
+        public unsafe void LaregeBuffersAreNotPooled_OfBigValueType()
         {
-            byte[] data = PixelDataPool<byte>.Rent(16384);
+            const int mb128 = 128 * 1024 * 1024;
+            int count = mb128 / sizeof(TestStruct);
 
-            Assert.True(data.Length >= 16384);
+            Assert.False(this.CheckIsPooled<TestStruct>(2, count));
         }
     }
 }