Skip to content

Commit ff36e83

Browse files
Add AllocationTrackedMemoryManager and refactor allocators (#3120)
* Add AllocationTrackedMemoryManager and refactor allocators * Add AllocationTrackingState and refactor tracking * Cleanup * Address feedback * Introduce ApplyOptions and use in allocators * Propagate allocation tracking to lifetime guards * Address Copilot feedback * Fix multi-buffer group tracking and enforce limits * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix override accesibility --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent c795d81 commit ff36e83

23 files changed

Lines changed: 744 additions & 92 deletions
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright (c) Six Labors.
2+
// Licensed under the Six Labors Split License.
3+
4+
using System.Buffers;
5+
6+
namespace SixLabors.ImageSharp.Memory;
7+
8+
/// <summary>
9+
/// Provides the tracked memory-owner contract required by <see cref="MemoryAllocator"/>.
10+
/// </summary>
11+
/// <typeparam name="T">The element type.</typeparam>
12+
/// <remarks>
13+
/// Custom allocators implement <see cref="MemoryAllocator.AllocateCore{T}(int, AllocationOptions)"/>
14+
/// and return a derived type. The base allocator attaches allocation tracking after the owner has been
15+
/// created so custom implementations cannot forget, duplicate, or mismatch the reservation lifecycle.
16+
/// </remarks>
17+
public abstract class AllocationTrackedMemoryManager<T> : MemoryManager<T>
18+
where T : struct
19+
{
20+
private AllocationTrackingState allocationTracking;
21+
22+
/// <summary>
23+
/// Releases resources held by the concrete tracked owner.
24+
/// </summary>
25+
/// <param name="disposing">
26+
/// <see langword="true"/> when the owner is being disposed deterministically;
27+
/// otherwise, <see langword="false"/>.
28+
/// </param>
29+
/// <remarks>
30+
/// Implementations release their own resources here. Allocation tracking is released by the sealed base
31+
/// dispose path after this method returns.
32+
/// </remarks>
33+
protected abstract void DisposeCore(bool disposing);
34+
35+
/// <inheritdoc />
36+
protected sealed override void Dispose(bool disposing)
37+
{
38+
try
39+
{
40+
this.DisposeCore(disposing);
41+
}
42+
finally
43+
{
44+
this.ReleaseAllocationTracking();
45+
}
46+
}
47+
48+
/// <summary>
49+
/// Attaches allocation tracking to this owner after allocation has succeeded.
50+
/// </summary>
51+
/// <param name="allocator">The allocator that owns the reservation for this instance.</param>
52+
/// <param name="lengthInBytes">The reserved allocation size, in bytes.</param>
53+
/// <remarks>
54+
/// <see cref="MemoryAllocator"/> calls this exactly once after <c>AllocateCore</c> returns.
55+
/// Derived allocators should not call it themselves; they only construct the concrete owner.
56+
/// </remarks>
57+
protected internal virtual void AttachAllocationTracking(MemoryAllocator allocator, long lengthInBytes)
58+
=> this.allocationTracking.Attach(allocator, lengthInBytes);
59+
60+
/// <summary>
61+
/// Releases any tracked allocation bytes associated with this instance.
62+
/// </summary>
63+
/// <remarks>
64+
/// Calling this more than once is safe; only the first call after tracking has been attached releases bytes.
65+
/// </remarks>
66+
private void ReleaseAllocationTracking() => this.allocationTracking.Release();
67+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) Six Labors.
2+
// Licensed under the Six Labors Split License.
3+
4+
namespace SixLabors.ImageSharp.Memory;
5+
6+
/// <summary>
7+
/// Tracks a single allocator reservation and releases it exactly once.
8+
/// </summary>
9+
/// <remarks>
10+
/// This type is intended to live as a mutable field on the owning object. It should not be copied
11+
/// after tracking has been attached, because the owner relies on a single shared release state.
12+
/// </remarks>
13+
internal struct AllocationTrackingState
14+
{
15+
private MemoryAllocator? allocator;
16+
private long lengthInBytes;
17+
private int released;
18+
19+
/// <summary>
20+
/// Attaches allocator reservation tracking to the current owner.
21+
/// </summary>
22+
/// <param name="allocator">The allocator that owns the reservation.</param>
23+
/// <param name="lengthInBytes">The reserved allocation size, in bytes.</param>
24+
/// <remarks>
25+
/// Must complete-before the owning object's reference is observable to any other thread.
26+
/// <see cref="MemoryAllocator"/> guarantees this by attaching synchronously on the allocating
27+
/// thread before returning the owner; reference publication then provides the release fence
28+
/// that makes these field writes visible to a subsequent <see cref="Release"/> on another thread.
29+
/// </remarks>
30+
internal void Attach(MemoryAllocator allocator, long lengthInBytes)
31+
{
32+
this.allocator = allocator;
33+
this.lengthInBytes = lengthInBytes;
34+
}
35+
36+
/// <summary>
37+
/// Releases the attached allocator reservation once.
38+
/// </summary>
39+
internal void Release()
40+
{
41+
if (Interlocked.Exchange(ref this.released, 1) == 0 && this.allocator != null)
42+
{
43+
this.allocator.ReleaseAccumulatedBytes(this.lengthInBytes);
44+
this.allocator = null;
45+
}
46+
}
47+
}
Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
1-
// Copyright (c) Six Labors.
1+
// Copyright (c) Six Labors.
22
// Licensed under the Six Labors Split License.
33

44
namespace SixLabors.ImageSharp.Memory;
55

6+
/// <summary>
7+
/// Provides helper methods for working with <see cref="AllocationOptions"/>.
8+
/// </summary>
69
internal static class AllocationOptionsExtensions
710
{
8-
public static bool Has(this AllocationOptions options, AllocationOptions flag) => (options & flag) == flag;
11+
/// <summary>
12+
/// Returns a value indicating whether the specified flag is set on the allocation options.
13+
/// </summary>
14+
/// <param name="options">The allocation options to inspect.</param>
15+
/// <param name="flag">The flag to test for.</param>
16+
/// <returns><see langword="true"/> if <paramref name="flag"/> is set; otherwise, <see langword="false"/>.</returns>
17+
public static bool Has(this AllocationOptions options, AllocationOptions flag)
18+
=> (options & flag) == flag;
919
}

src/ImageSharp/Memory/Allocators/Internals/BasicArrayBuffer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public BasicArrayBuffer(T[] array)
4747
public override Span<T> GetSpan() => this.Array.AsSpan(0, this.Length);
4848

4949
/// <inheritdoc />
50-
protected override void Dispose(bool disposing)
50+
protected override void DisposeCore(bool disposing)
5151
{
5252
}
5353

src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Memory.Internals;
1111
/// Provides a base class for <see cref="IMemoryOwner{T}"/> implementations by implementing pinning logic for <see cref="MemoryManager{T}"/> adaption.
1212
/// </summary>
1313
/// <typeparam name="T">The element type.</typeparam>
14-
internal abstract class ManagedBufferBase<T> : MemoryManager<T>
14+
internal abstract class ManagedBufferBase<T> : AllocationTrackedMemoryManager<T>
1515
where T : struct
1616
{
1717
private GCHandle pinHandle;

src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ namespace SixLabors.ImageSharp.Memory.Internals;
1111
/// </summary>
1212
internal abstract class RefCountedMemoryLifetimeGuard : IDisposable
1313
{
14+
private AllocationTrackingState allocationTracking;
1415
private int refCount = 1;
1516
private int disposed;
1617
private int released;
@@ -38,6 +39,14 @@ protected RefCountedMemoryLifetimeGuard()
3839

3940
public void ReleaseRef() => this.ReleaseRef(false);
4041

42+
/// <summary>
43+
/// Attaches allocator reservation tracking to this lifetime guard.
44+
/// </summary>
45+
/// <param name="allocator">The allocator that owns the reservation.</param>
46+
/// <param name="lengthInBytes">The reserved allocation size, in bytes.</param>
47+
public void AttachAllocationTracking(MemoryAllocator allocator, long lengthInBytes)
48+
=> this.allocationTracking.Attach(allocator, lengthInBytes);
49+
4150
public void Dispose()
4251
{
4352
int wasDisposed = Interlocked.Exchange(ref this.disposed, 1);
@@ -69,6 +78,10 @@ private void ReleaseRef(bool finalizing)
6978
}
7079

7180
this.Release();
81+
82+
// Guard-backed resources can be recovered by finalization, so their allocator
83+
// reservation must follow the guard's actual release point instead of the owner object.
84+
this.allocationTracking.Release();
7285
}
7386
}
7487
}

src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ internal class SharedArrayPoolBuffer<T> : ManagedBufferBase<T>, IRefCounted
1313
where T : struct
1414
{
1515
private readonly int lengthInBytes;
16-
private LifetimeGuard lifetimeGuard;
16+
private readonly LifetimeGuard lifetimeGuard;
1717

1818
public SharedArrayPoolBuffer(int lengthInElements)
1919
{
@@ -24,7 +24,10 @@ public SharedArrayPoolBuffer(int lengthInElements)
2424

2525
public byte[]? Array { get; private set; }
2626

27-
protected override void Dispose(bool disposing)
27+
protected internal override void AttachAllocationTracking(MemoryAllocator allocator, long lengthInBytes)
28+
=> this.lifetimeGuard.AttachAllocationTracking(allocator, lengthInBytes);
29+
30+
protected override void DisposeCore(bool disposing)
2831
{
2932
if (this.Array == null)
3033
{

src/ImageSharp/Memory/Allocators/Internals/UnmanagedBuffer{T}.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Memory.Internals;
1212
/// access to unmanaged buffers allocated by <see cref="Marshal.AllocHGlobal(int)"/>.
1313
/// </summary>
1414
/// <typeparam name="T">The element type.</typeparam>
15-
internal sealed unsafe class UnmanagedBuffer<T> : MemoryManager<T>, IRefCounted
15+
internal sealed unsafe class UnmanagedBuffer<T> : AllocationTrackedMemoryManager<T>, IRefCounted
1616
where T : struct
1717
{
1818
private readonly int lengthInElements;
@@ -31,6 +31,9 @@ public UnmanagedBuffer(int lengthInElements, UnmanagedBufferLifetimeGuard lifeti
3131

3232
public void* Pointer => this.lifetimeGuard.Handle.Pointer;
3333

34+
protected internal override void AttachAllocationTracking(MemoryAllocator allocator, long lengthInBytes)
35+
=> this.lifetimeGuard.AttachAllocationTracking(allocator, lengthInBytes);
36+
3437
public override Span<T> GetSpan()
3538
{
3639
DebugGuard.NotDisposed(this.disposed == 1, this.GetType().Name);
@@ -52,7 +55,7 @@ public override MemoryHandle Pin(int elementIndex = 0)
5255
}
5356

5457
/// <inheritdoc />
55-
protected override void Dispose(bool disposing)
58+
protected override void DisposeCore(bool disposing)
5659
{
5760
DebugGuard.IsTrue(disposing, nameof(disposing), "Unmanaged buffers should not have finalizer!");
5861

0 commit comments

Comments
 (0)