polish MemoryAllocator API

src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.CommonFactoryMethods.cs

@@ -29,6 +29,9 @@ namespace SixLabors.ImageSharp.Memory
         /// </summary>
         private const int DefaultNormalPoolBucketCount = 16;
 
+        // TODO: This value should be determined by benchmarking
+        private const int DefaultBufferCapacityInBytes = int.MaxValue / 4;
+
         /// <summary>
         /// This is the default. Should be good for most use cases.
         /// </summary>
@@ -39,7 +42,8 @@ namespace SixLabors.ImageSharp.Memory
                 DefaultMaxPooledBufferSizeInBytes,
                 DefaultBufferSelectorThresholdInBytes,
                 DefaultLargePoolBucketCount,
-                DefaultNormalPoolBucketCount);
+                DefaultNormalPoolBucketCount,
+                DefaultBufferCapacityInBytes);
         }
 
         /// <summary>
@@ -69,4 +73,4 @@ namespace SixLabors.ImageSharp.Memory
             return new ArrayPoolMemoryAllocator(128 * 1024 * 1024, 32 * 1024 * 1024, 16, 32);
         }
     }
-}
+}

src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.cs

@@ -60,13 +60,41 @@ namespace SixLabors.ImageSharp.Memory
         /// <param name="poolSelectorThresholdInBytes">The threshold to pool arrays in <see cref="largeArrayPool"/> which has less buckets for memory safety.</param>
         /// <param name="maxArraysPerBucketLargePool">Max arrays per bucket for the large array pool.</param>
         /// <param name="maxArraysPerBucketNormalPool">Max arrays per bucket for the normal array pool.</param>
-        public ArrayPoolMemoryAllocator(int maxPoolSizeInBytes, int poolSelectorThresholdInBytes, int maxArraysPerBucketLargePool, int maxArraysPerBucketNormalPool)
+        public ArrayPoolMemoryAllocator(
+            int maxPoolSizeInBytes,
+            int poolSelectorThresholdInBytes,
+            int maxArraysPerBucketLargePool,
+            int maxArraysPerBucketNormalPool)
+            : this(
+                maxPoolSizeInBytes,
+                poolSelectorThresholdInBytes,
+                maxArraysPerBucketLargePool,
+                maxArraysPerBucketNormalPool,
+                DefaultBufferCapacityInBytes)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ArrayPoolMemoryAllocator"/> class.
+        /// </summary>
+        /// <param name="maxPoolSizeInBytes">The maximum size of pooled arrays. Arrays over the thershold are gonna be always allocated.</param>
+        /// <param name="poolSelectorThresholdInBytes">The threshold to pool arrays in <see cref="largeArrayPool"/> which has less buckets for memory safety.</param>
+        /// <param name="maxArraysPerBucketLargePool">Max arrays per bucket for the large array pool.</param>
+        /// <param name="maxArraysPerBucketNormalPool">Max arrays per bucket for the normal array pool.</param>
+        /// <param name="bufferCapacityInBytes">The length of the largest contiguous buffer that can be handled by this allocator instance.</param>
+        public ArrayPoolMemoryAllocator(
+            int maxPoolSizeInBytes,
+            int poolSelectorThresholdInBytes,
+            int maxArraysPerBucketLargePool,
+            int maxArraysPerBucketNormalPool,
+            int bufferCapacityInBytes)
         {
             Guard.MustBeGreaterThan(maxPoolSizeInBytes, 0, nameof(maxPoolSizeInBytes));
             Guard.MustBeLessThanOrEqualTo(poolSelectorThresholdInBytes, maxPoolSizeInBytes, nameof(poolSelectorThresholdInBytes));
 
             this.MaxPoolSizeInBytes = maxPoolSizeInBytes;
             this.PoolSelectorThresholdInBytes = poolSelectorThresholdInBytes;
+            this.BufferCapacityInBytes = bufferCapacityInBytes;
             this.maxArraysPerBucketLargePool = maxArraysPerBucketLargePool;
             this.maxArraysPerBucketNormalPool = maxArraysPerBucketNormalPool;
 
@@ -84,9 +112,9 @@ namespace SixLabors.ImageSharp.Memory
         public int PoolSelectorThresholdInBytes { get; }
 
         /// <summary>
-        /// Gets or sets the length of the largest contiguous buffer that can be handled by this allocator instance.
+        /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance.
         /// </summary>
-        public int BufferCapacityInBytes { get; set; } = DefaultBufferCapacity;
+        public int BufferCapacityInBytes { get; internal set; } // Setter is internal for easy configuration in tests
 
         /// <inheritdoc />
         public override void ReleaseRetainedResources()
@@ -103,11 +131,10 @@ namespace SixLabors.ImageSharp.Memory
             Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length));
             int itemSizeBytes = Unsafe.SizeOf<T>();
             int bufferSizeInBytes = length * itemSizeBytes;
-            if (bufferSizeInBytes < 0)
+            if (bufferSizeInBytes < 0 || bufferSizeInBytes > BufferCapacityInBytes)
             {
-                throw new ArgumentOutOfRangeException(
+                throw new InvalidMemoryOperationException(
-                    nameof(length),
+                    $"Requested allocation {length} elements of {typeof(T).Name} is over the capacity of the MemoryAllocator.");
-                    $"{nameof(ArrayPoolMemoryAllocator)} can not allocate {length} elements of {typeof(T).Name}.");
             }
 
             ArrayPool<byte> pool = this.GetArrayPool(bufferSizeInBytes);

src/ImageSharp/Memory/Allocators/MemoryAllocator.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
+using System;
 using System.Buffers;
 
 namespace SixLabors.ImageSharp.Memory
@@ -10,7 +11,7 @@ namespace SixLabors.ImageSharp.Memory
     /// </summary>
     public abstract class MemoryAllocator
     {
-        internal const int DefaultBufferCapacity = int.MaxValue / 2;
+
 
         /// <summary>
         /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes.
@@ -25,6 +26,8 @@ namespace SixLabors.ImageSharp.Memory
         /// <param name="length">Size of the buffer to allocate.</param>
         /// <param name="options">The allocation options.</param>
         /// <returns>A buffer of values of type <typeparamref name="T"/>.</returns>
+        /// <exception cref="ArgumentOutOfRangeException">When length is zero or negative.</exception>
+        /// <exception cref="InvalidMemoryOperationException">When length is over the capacity of the allocator.</exception>
         public abstract IMemoryOwner<T> Allocate<T>(int length, AllocationOptions options = AllocationOptions.None)
             where T : struct;
 
@@ -34,6 +37,8 @@ namespace SixLabors.ImageSharp.Memory
         /// <param name="length">The requested buffer length.</param>
         /// <param name="options">The allocation options.</param>
         /// <returns>The <see cref="IManagedByteBuffer"/>.</returns>
+        /// <exception cref="ArgumentOutOfRangeException">When length is zero or negative.</exception>
+        /// <exception cref="InvalidMemoryOperationException">When length is over the capacity of the allocator.</exception>
         public abstract IManagedByteBuffer AllocateManagedByteBuffer(int length, AllocationOptions options = AllocationOptions.None);
 
         /// <summary>

src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs

@@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Memory
     public sealed class SimpleGcMemoryAllocator : MemoryAllocator
     {
         /// <inheritdoc />
-        protected internal override int GetBufferCapacityInBytes() => DefaultBufferCapacity;
+        protected internal override int GetBufferCapacityInBytes() => int.MaxValue;
 
         /// <inheritdoc />
         public override IMemoryOwner<T> Allocate<T>(int length, AllocationOptions options = AllocationOptions.None)

tests/ImageSharp.Tests/Memory/Alocators/ArrayPoolMemoryAllocatorTests.cs → tests/ImageSharp.Tests/Memory/Allocators/ArrayPoolMemoryAllocatorTests.cs

@@ -1,18 +1,15 @@
 // Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
-// ReSharper disable InconsistentNaming
 using System;
 using System.Buffers;
-using System.Diagnostics;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using Microsoft.DotNet.RemoteExecutor;
-using Microsoft.Win32;
+using SixLabors.ImageSharp.Memory;
-using SixLabors.ImageSharp.Tests;
 using Xunit;
 
-namespace SixLabors.ImageSharp.Memory.Tests
+namespace SixLabors.ImageSharp.Tests.Memory.Allocators
 {
     public class ArrayPoolMemoryAllocatorTests
     {
@@ -116,13 +113,13 @@ namespace SixLabors.ImageSharp.Memory.Tests
             MemoryAllocator memoryAllocator = this.LocalFixture.MemoryAllocator;
             using (IMemoryOwner<int> firstAlloc = memoryAllocator.Allocate<int>(42))
             {
-                firstAlloc.GetSpan().Fill(666);
+                BufferExtensions.GetSpan(firstAlloc).Fill(666);
             }
 
             using (IMemoryOwner<int> secondAlloc = memoryAllocator.Allocate<int>(42, options))
             {
                 int expected = options == AllocationOptions.Clean ? 0 : 666;
-                Assert.Equal(expected, secondAlloc.GetSpan()[0]);
+                Assert.Equal(expected, BufferExtensions.GetSpan(secondAlloc)[0]);
             }
         }
 
@@ -133,7 +130,7 @@ namespace SixLabors.ImageSharp.Memory.Tests
         {
             MemoryAllocator memoryAllocator = this.LocalFixture.MemoryAllocator;
             IMemoryOwner<int> buffer = memoryAllocator.Allocate<int>(32);
-            ref int ptrToPrev0 = ref MemoryMarshal.GetReference(buffer.GetSpan());
+            ref int ptrToPrev0 = ref MemoryMarshal.GetReference(BufferExtensions.GetSpan(buffer));
 
             if (!keepBufferAlive)
             {
@@ -144,7 +141,7 @@ namespace SixLabors.ImageSharp.Memory.Tests
 
             buffer = memoryAllocator.Allocate<int>(32);
 
-            Assert.False(Unsafe.AreSame(ref ptrToPrev0, ref buffer.GetReference()));
+            Assert.False(Unsafe.AreSame(ref ptrToPrev0, ref BufferExtensions.GetReference(buffer)));
         }
 
         [Fact]
@@ -164,12 +161,12 @@ namespace SixLabors.ImageSharp.Memory.Tests
                 const int ArrayLengthThreshold = PoolSelectorThresholdInBytes / sizeof(int);
 
                 IMemoryOwner<int> small = StaticFixture.MemoryAllocator.Allocate<int>(ArrayLengthThreshold - 1);
-                ref int ptr2Small = ref small.GetReference();
+                ref int ptr2Small = ref BufferExtensions.GetReference(small);
                 small.Dispose();
 
                 IMemoryOwner<int> large = StaticFixture.MemoryAllocator.Allocate<int>(ArrayLengthThreshold + 1);
 
-                Assert.False(Unsafe.AreSame(ref ptr2Small, ref large.GetReference()));
+                Assert.False(Unsafe.AreSame(ref ptr2Small, ref BufferExtensions.GetReference(large)));
             }
 
             RemoteExecutor.Invoke(RunTest).Dispose();
@@ -216,14 +213,34 @@ namespace SixLabors.ImageSharp.Memory.Tests
 
         [Theory]
         [InlineData(-1)]
-        [InlineData((int.MaxValue / SizeOfLargeStruct) + 1)]
+        [InlineData(-111)]
-        public void AllocateIncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length)
+        public void Allocate_Negative_Throws_ArgumentOutOfRangeException(int length)
         {
             ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(() =>
                 this.LocalFixture.MemoryAllocator.Allocate<LargeStruct>(length));
             Assert.Equal("length", ex.ParamName);
         }
 
+        [Fact]
+        public void AllocateZero()
+        {
+            using IMemoryOwner<int> buffer = this.LocalFixture.MemoryAllocator.Allocate<int>(0);
+            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<InvalidMemoryOperationException>(() =>
+                this.LocalFixture.MemoryAllocator.Allocate<LargeStruct>(length));
+        }
+
         [Theory]
         [InlineData(-1)]
         public void AllocateManagedByteBuffer_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length)
@@ -235,7 +252,7 @@ namespace SixLabors.ImageSharp.Memory.Tests
 
         private class MemoryAllocatorFixture
         {
-            public MemoryAllocator MemoryAllocator { get; set; } =
+            public ArrayPoolMemoryAllocator MemoryAllocator { get; set; } =
                 new ArrayPoolMemoryAllocator(MaxPooledBufferSizeInBytes, PoolSelectorThresholdInBytes);
 
             /// <summary>
@@ -245,11 +262,11 @@ namespace SixLabors.ImageSharp.Memory.Tests
                 where T : struct
             {
                 IMemoryOwner<T> buffer = this.MemoryAllocator.Allocate<T>(length);
-                ref T ptrToPrevPosition0 = ref buffer.GetReference();
+                ref T ptrToPrevPosition0 = ref BufferExtensions.GetReference(buffer);
                 buffer.Dispose();
 
                 buffer = this.MemoryAllocator.Allocate<T>(length);
-                bool sameBuffers = Unsafe.AreSame(ref ptrToPrevPosition0, ref buffer.GetReference());
+                bool sameBuffers = Unsafe.AreSame(ref ptrToPrevPosition0, ref BufferExtensions.GetReference(buffer));
                 buffer.Dispose();
 
                 return sameBuffers;

tests/ImageSharp.Tests/Memory/Alocators/BufferExtensions.cs → tests/ImageSharp.Tests/Memory/Allocators/BufferExtensions.cs

@@ -6,7 +6,7 @@ using System.Buffers;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 
-namespace SixLabors.ImageSharp.Memory.Tests
+namespace SixLabors.ImageSharp.Tests.Memory.Allocators
 {
     internal static class BufferExtensions
     {
@@ -22,4 +22,4 @@ namespace SixLabors.ImageSharp.Memory.Tests
             where T : struct =>
             ref MemoryMarshal.GetReference(buffer.GetSpan());
     }
-}
+}

tests/ImageSharp.Tests/Memory/Alocators/BufferTestSuite.cs → tests/ImageSharp.Tests/Memory/Allocators/BufferTestSuite.cs

@@ -5,10 +5,11 @@ using System;
 using System.Buffers;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using SixLabors.ImageSharp.Memory;
 using Xunit;
 
 // ReSharper disable InconsistentNaming
-namespace SixLabors.ImageSharp.Memory.Tests
+namespace SixLabors.ImageSharp.Tests.Memory.Allocators
 {
     /// <summary>
     /// Inherit this class to test an <see cref="IMemoryOwner{T}"/> implementation (provided by <see cref="MemoryAllocator"/>).

tests/ImageSharp.Tests/Memory/Alocators/SimpleGcMemoryAllocatorTests.cs → tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs

@@ -3,9 +3,10 @@
 
 using System;
 using System.Runtime.InteropServices;
+using SixLabors.ImageSharp.Memory;
 using Xunit;
 
-namespace SixLabors.ImageSharp.Memory.Tests
+namespace SixLabors.ImageSharp.Tests.Memory.Allocators
 {
     public class SimpleGcMemoryAllocatorTests
     {