Merge branch 'master' into js/matrix-filters

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
 {
@@ -14,9 +14,35 @@ namespace SixLabors.ImageSharp.Memory
         where T : struct
     {
         /// <summary>
-        /// The <see cref="ArrayPool{T}"/> which is not kept clean.
+        /// 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 static readonly ArrayPool<T> ArrayPool = ArrayPool<T>.Create(CalculateMaxArrayLength(), 50);
+        internal const int MaxPooledBufferSizeInBytes = 32 * 1024 * 1024;
+
+        /// <summary>
+        /// The threshold to pool arrays in <see cref="LargeArrayPool"/> which has less buckets for memory safety.
+        /// </summary>
+        private const int LargeBufferThresholdInBytes = 8 * 1024 * 1024;
+
+        /// <summary>
+        /// The maximum array length of the <see cref="LargeArrayPool"/>.
+        /// </summary>
+        private static readonly int MaxLargeArrayLength = MaxPooledBufferSizeInBytes / Unsafe.SizeOf<T>();
+
+        /// <summary>
+        /// The maximum array length of the <see cref="NormalArrayPool"/>.
+        /// </summary>
+        private static readonly int MaxNormalArrayLength = LargeBufferThresholdInBytes / Unsafe.SizeOf<T>();
+
+        /// <summary>
+        /// The <see cref="ArrayPool{T}"/> for huge buffers, which is not kept clean.
+        /// </summary>
+        private static readonly ArrayPool<T> LargeArrayPool = ArrayPool<T>.Create(MaxLargeArrayLength, 8);
+
+        /// <summary>
+        /// The <see cref="ArrayPool{T}"/> for small-to-medium buffers which is not kept clean.
+        /// </summary>
+        private static readonly ArrayPool<T> NormalArrayPool = ArrayPool<T>.Create(MaxNormalArrayLength, 24);
 
         /// <summary>
         /// Rents the pixel array from the pool.
@@ -25,7 +51,14 @@ namespace SixLabors.ImageSharp.Memory
         /// <returns>The <see cref="T:TPixel[]"/></returns>
         public static T[] Rent(int minimumLength)
         {
-            return ArrayPool.Rent(minimumLength);
+            if (minimumLength <= MaxNormalArrayLength)
+            {
+                return NormalArrayPool.Rent(minimumLength);
+            }
+            else
+            {
+                return LargeArrayPool.Rent(minimumLength);
+            }
         }
 
         /// <summary>
@@ -34,25 +67,13 @@ namespace SixLabors.ImageSharp.Memory
         /// <param name="array">The array to return to the buffer pool.</param>
         public static void Return(T[] array)
         {
-            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)
+            if (array.Length <= MaxNormalArrayLength)
             {
-                const int MaximumExpectedImageSize = 16384 * 16384;
-                return MaximumExpectedImageSize;
+                NormalArrayPool.Return(array);
             }
             else
             {
-                const int MaxArrayLength = 1024 * 1024; // Match default pool.
-                return MaxArrayLength;
+                LargeArrayPool.Return(array);
             }
         }
     }

tests/ImageSharp.Tests/Memory/PixelDataPoolTests.cs

@@ -1,26 +1,33 @@
 // 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;
+    using System;
 
     /// <summary>
     /// Tests the <see cref="PixelDataPool{T}"/> class.
     /// </summary>
     public class PixelDataPoolTests
     {
-        [Fact]
-        public void PixelDataPoolRentsMinimumSize()
+        private const int MaxPooledBufferSizeInBytes = PixelDataPool<byte>.MaxPooledBufferSizeInBytes;
+
+        readonly object monitor = new object();
+        
+        [Theory]
+        [InlineData(1)]
+        [InlineData(1024)]
+        public void PixelDataPoolRentsMinimumSize(int size)
         {
-            Rgba32[] pixels = PixelDataPool<Rgba32>.Rent(1024);
+            Rgba32[] pixels = PixelDataPool<Rgba32>.Rent(size);
 
-            Assert.True(pixels.Length >= 1024);
+            Assert.True(pixels.Length >= size);
         }
 
         [Fact]
@@ -33,23 +40,66 @@ namespace SixLabors.ImageSharp.Tests.Memory
             Assert.True(pixels.Length >= 1024);
         }
 
+        /// <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();
+            }
+        }
+
+        [Theory]
+        [InlineData(32)]
+        [InlineData(512)]
+        [InlineData(MaxPooledBufferSizeInBytes-1)]
+        public void SmallBuffersArePooled(int size)
+        {
+            Assert.True(this.CheckIsPooled<byte>(5, size));
+        }
+
         [Theory]
-        [InlineData(false)]
-        [InlineData(true)]
-        public void CalculateMaxArrayLength(bool isRawData)
+        [InlineData(128 * 1024 * 1024)]
+        [InlineData(MaxPooledBufferSizeInBytes+1)]
+        public void LargeBuffersAreNotPooled_OfByte(int size)
         {
-            int max = isRawData ? PixelDataPool<int>.CalculateMaxArrayLength()
-                          : PixelDataPool<Rgba32>.CalculateMaxArrayLength();
+            Assert.False(this.CheckIsPooled<byte>(2, size));
+        }
 
-            Assert.Equal(max > 1024 * 1024, !isRawData);
+        [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));
         }
     }
 }