From 2beb01baa6ee3bfa2b8199c20c7fc56a9afeced7 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 11 May 2026 20:23:30 +1000 Subject: [PATCH] Introduce ApplyOptions and use in allocators --- .../Memory/AllocationTrackingState.cs | 6 ++++ .../Memory/Allocators/MemoryAllocator.cs | 18 ++++++++--- .../Allocators/SimpleGcMemoryAllocator.cs | 14 +-------- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 9 ++++++ .../DiscontiguousBuffers/MemoryGroup{T}.cs | 2 +- ...niformUnmanagedPoolMemoryAllocatorTests.cs | 31 +++++++++++++++++++ 6 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Memory/AllocationTrackingState.cs b/src/ImageSharp/Memory/AllocationTrackingState.cs index a0c22ccebf..1e9a632ed8 100644 --- a/src/ImageSharp/Memory/AllocationTrackingState.cs +++ b/src/ImageSharp/Memory/AllocationTrackingState.cs @@ -21,6 +21,12 @@ internal struct AllocationTrackingState /// /// The allocator that owns the reservation. /// The reserved allocation size, in bytes. + /// + /// Must complete-before the owning object's reference is observable to any other thread. + /// guarantees this by attaching synchronously on the allocating + /// thread before returning the owner; reference publication then provides the release fence + /// that makes these field writes visible to a subsequent on another thread. + /// internal void Attach(MemoryAllocator allocator, long lengthInBytes) { this.allocator = allocator; diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 8ea6659e64..e08672baa8 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -83,18 +83,26 @@ public abstract class MemoryAllocator public static MemoryAllocator Create(MemoryAllocatorOptions options) { UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(options.MaximumPoolSizeMegabytes); + allocator.ApplyOptions(options); + return allocator; + } + + /// + /// Applies the supplied to this instance. + /// + /// The options to apply. Properties left as are ignored. + private protected void ApplyOptions(MemoryAllocatorOptions options) + { if (options.AllocationLimitMegabytes.HasValue) { - allocator.MemoryGroupAllocationLimitBytes = options.AllocationLimitMegabytes.Value * 1024L * 1024L; - allocator.SingleBufferAllocationLimitBytes = (int)Math.Min(allocator.SingleBufferAllocationLimitBytes, allocator.MemoryGroupAllocationLimitBytes); + this.MemoryGroupAllocationLimitBytes = options.AllocationLimitMegabytes.Value * 1024L * 1024L; + this.SingleBufferAllocationLimitBytes = (int)Math.Min(this.SingleBufferAllocationLimitBytes, this.MemoryGroupAllocationLimitBytes); } if (options.AccumulativeAllocationLimitMegabytes.HasValue) { - allocator.AccumulativeAllocationLimitBytes = options.AccumulativeAllocationLimitMegabytes.Value * 1024L * 1024L; + this.AccumulativeAllocationLimitBytes = options.AccumulativeAllocationLimitMegabytes.Value * 1024L * 1024L; } - - return allocator; } /// diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 162bc85cad..5d183fa442 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -22,19 +22,7 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// Initializes a new instance of the class with custom limits. /// /// The to apply. - public SimpleGcMemoryAllocator(MemoryAllocatorOptions options) - { - if (options.AllocationLimitMegabytes.HasValue) - { - this.MemoryGroupAllocationLimitBytes = options.AllocationLimitMegabytes.Value * 1024L * 1024L; - this.SingleBufferAllocationLimitBytes = (int)Math.Min(this.SingleBufferAllocationLimitBytes, this.MemoryGroupAllocationLimitBytes); - } - - if (options.AccumulativeAllocationLimitMegabytes.HasValue) - { - this.AccumulativeAllocationLimitBytes = options.AccumulativeAllocationLimitMegabytes.Value * 1024L * 1024L; - } - } + public SimpleGcMemoryAllocator(MemoryAllocatorOptions options) => this.ApplyOptions(options); /// protected internal override int GetBufferCapacityInBytes() => int.MaxValue; diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index ad485f9271..cfffc679a2 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -70,6 +70,15 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } + internal UniformUnmanagedMemoryPoolMemoryAllocator( + int sharedArrayPoolThresholdInBytes, + int poolBufferSizeInBytes, + long maxPoolSizeInBytes, + int unmanagedBufferSizeInBytes, + MemoryAllocatorOptions options) + : this(sharedArrayPoolThresholdInBytes, poolBufferSizeInBytes, maxPoolSizeInBytes, unmanagedBufferSizeInBytes) + => this.ApplyOptions(options); + /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 870c852d50..0aa09beb9e 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -102,8 +102,8 @@ internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable int bufferAlignmentInElements, AllocationOptions options = AllocationOptions.None) { - int bufferCapacityInBytes = allocator.GetBufferCapacityInBytes(); Guard.NotNull(allocator, nameof(allocator)); + int bufferCapacityInBytes = allocator.GetBufferCapacityInBytes(); if (totalLengthInElements < 0) { diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 06da3505e1..d440714fa9 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -477,6 +477,37 @@ public class UniformUnmanagedPoolMemoryAllocatorTests allocator.AllocateGroup(oneMb, 1024).Dispose(); } + [Fact] + public void AllocateGroup_AccumulativeLimit_NonPoolFallback_TracksOncePerGroup() + { + // Configure the pool with zero capacity so multi-segment requests bypass both the + // single-buffer-from-pool path and MemoryGroup.TryAllocate(pool, ...) and fall + // through to MemoryGroup.Allocate(nonPoolAllocator, ...). The unmanaged segment + // size is small enough that the request must span multiple segments, which is the + // path where per-segment double-counting could regress. + UniformUnmanagedMemoryPoolMemoryAllocator allocator = new( + sharedArrayPoolThresholdInBytes: 64 * 1024, + poolBufferSizeInBytes: 128 * 1024, + maxPoolSizeInBytes: 0, + unmanagedBufferSizeInBytes: 256 * 1024, + new MemoryAllocatorOptions { AccumulativeAllocationLimitMegabytes = 1 }); + + // 768 KB exceeds the pool buffer size, so the request takes the multi-segment + // non-pool fallback (three 256 KB segments). If tracking double-counted (group + // plus each segment), reservation would be 768 KB + 768 KB = 1.5 MB and exceed + // the 1 MB limit on allocation itself. + MemoryGroup g = allocator.AllocateGroup(768 * 1024, 1024); + Assert.True(g.Count > 1, "Test setup must exercise the multi-segment fallback path."); + + // Reservation should be exactly 768 KB; another 512 KB would push to 1.25 MB and throw. + Assert.Throws(() => allocator.Allocate(512 * 1024)); + + g.Dispose(); + + // After disposal the reservation is fully released; a second equivalent group succeeds. + allocator.AllocateGroup(768 * 1024, 1024).Dispose(); + } + [ConditionalFact(typeof(Environment), nameof(Environment.Is64BitProcess))] public void MemoryAllocator_Create_SetHighLimit() {