tsai
/
ImageSharp
mirror of https://github.com/SixLabors/ImageSharp
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Make UnmanagedMemoryHandle members readonly and improve pool finalization tests

pull/3040/head
James Jackson-South 4 weeks ago
parent
302700050b
commit
bca6197517
2 changed files with 90 additions and 86 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 14
      src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs
  2. 162
      tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs

14
src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs
View File

@ -39,13 +39,13 @@ internal struct UnmanagedMemoryHandle : IEquatable<UnmanagedMemoryHandle>
Interlocked.Increment(ref totalOutstandingHandles);
}
public IntPtr Handle => this.handle;
public readonly IntPtr Handle => this.handle;
public bool IsInvalid => this.Handle == IntPtr.Zero;
public readonly bool IsInvalid => this.Handle == IntPtr.Zero;
public bool IsValid => this.Handle != IntPtr.Zero;
public readonly bool IsValid => this.Handle != IntPtr.Zero;
public unsafe void* Pointer => (void*)this.Handle;
public readonly unsafe void* Pointer => (void*)this.Handle;
/// <summary>
/// Gets the total outstanding handle allocations for testing purposes.
@ -121,9 +121,9 @@ internal struct UnmanagedMemoryHandle : IEquatable<UnmanagedMemoryHandle>
this.lengthInBytes = 0;
}
public bool Equals(UnmanagedMemoryHandle other) => this.handle.Equals(other.handle);
public readonly bool Equals(UnmanagedMemoryHandle other) => this.handle.Equals(other.handle);
public override bool Equals(object? obj) => obj is UnmanagedMemoryHandle other && this.Equals(other);
public override readonly bool Equals(object? obj) => obj is UnmanagedMemoryHandle other && this.Equals(other);
public override int GetHashCode() => this.handle.GetHashCode();
public override readonly int GetHashCode() => this.handle.GetHashCode();
}

162
tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs
View File

@ -2,6 +2,7 @@
// Licensed under the Six Labors Split License.
using System.Buffers;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Microsoft.DotNet.RemoteExecutor;
@ -273,67 +274,76 @@ public class UniformUnmanagedPoolMemoryAllocatorTests
[InlineData(1200)] // Group of two UniformUnmanagedMemoryPool buffers
public void AllocateMemoryGroup_Finalization_ReturnsToPool(int length)
{
if (TestEnvironment.IsMacOS)
{
// Skip on macOS: https://github.com/SixLabors/ImageSharp/issues/1887
return;
}
if (TestEnvironment.OSArchitecture == Architecture.Arm64)
{
// Skip on ARM64: https://github.com/SixLabors/ImageSharp/issues/2342
return;
}
if (!TestEnvironment.RunsOnCI)
{
// This may fail in local runs resulting in high memory load.
// Remove the condition for local debugging!
return;
}
// RunTest(length.ToString());
RemoteExecutor.Invoke(RunTest, length.ToString()).Dispose();
RemoteExecutor.Invoke(RunTest, length.ToString(CultureInfo.InvariantCulture)).Dispose();
static void RunTest(string lengthStr)
{
UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(512, 1024, 16 * 1024, 1024);
int lengthInner = int.Parse(lengthStr);
int lengthInner = int.Parse(lengthStr, CultureInfo.InvariantCulture);
// We want to verify that a leaked (not disposed) `MemoryGroup<byte>` still returns its
// unmanaged handles into the pool when it is finalized.
//
// We intentionally do NOT validate this by checking the contents of the re-rented memory
// (contents are not guaranteed to be preserved) nor by comparing pointer values
// (the pool may return a different handle while still correctly pooling).
//
// Instead, we validate that after a forced GC+finalization cycle, a subsequent allocation
// of the same size does not cause the number of outstanding unmanaged handles to *increase*
// compared to a known baseline.
// Establish a baseline: create one allocation and dispose it so the pool is initialized.
// (This ensures subsequent observations are not biased by first-time pool growth.)
allocator.AllocateGroup<byte>(lengthInner, 100).Dispose();
int baselineHandles = UnmanagedMemoryHandle.TotalOutstandingHandles;
// Leak one allocation and force finalization.
AllocateGroupAndForget(allocator, lengthInner);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
AllocateGroupAndForget(allocator, lengthInner, true);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
using MemoryGroup<byte> g = allocator.AllocateGroup<byte>(lengthInner, 100);
Assert.Equal(42, g.First().Span[0]);
// Allocate again. If the leaked group was finalized correctly and returned to the pool,
// this should not require additional unmanaged allocations (ie, the handle count must not grow).
allocator.AllocateGroup<byte>(lengthInner, 100).Dispose();
// Note: we use "<=" instead of "==" here.
//
// After we record the baseline, the pool is allowed to legitimately *decrease*
// `UnmanagedMemoryHandle.TotalOutstandingHandles` by trimming retained buffers
// (eg. via the pool's trim timer/GC callbacks/high-pressure logic).
//
// What must not happen is the opposite: the leaked (non-disposed) group should be finalized
// and its handles returned to the pool such that allocating again does NOT require creating
// additional unmanaged handles. Therefore the only invariant we can reliably assert here is
// "no growth" relative to the baseline.
Assert.True(UnmanagedMemoryHandle.TotalOutstandingHandles <= baselineHandles);
GC.KeepAlive(allocator);
}
}
private static void AllocateGroupAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length, bool check = false)
[MethodImpl(MethodImplOptions.NoInlining)]
private static void AllocateGroupAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length)
{
// Allocate a group and drop the reference without disposing.
// The test relies on the group's finalizer to return the rented memory to the pool.
MemoryGroup<byte> g = allocator.AllocateGroup<byte>(length, 100);
if (check)
{
Assert.Equal(42, g.First().Span[0]);
}
g.First().Span[0] = 42;
// Touch the memory to ensure the buffer is actually materialized/usable.
g[0].Span[0] = 42;
if (length < 512)
{
// For ArrayPool.Shared, first array will be returned to the TLS storage of the finalizer thread,
// repeat rental to make sure per-core buckets are also utilized.
// For ArrayPool.Shared, the first rented array may be stored in TLS on the finalizer thread.
// Repeat rental to increase the chance that per-core buckets are involved when length
// is small and allocations go through ArrayPool.
MemoryGroup<byte> g1 = allocator.AllocateGroup<byte>(length, 100);
g1.First().Span[0] = 42;
g1[0].Span[0] = 42;
g1 = null;
}
g = null;
}
[Theory]
@ -341,69 +351,63 @@ public class UniformUnmanagedPoolMemoryAllocatorTests
[InlineData(600)] // Group of single UniformUnmanagedMemoryPool buffer
public void AllocateSingleMemoryOwner_Finalization_ReturnsToPool(int length)
{
if (TestEnvironment.IsMacOS)
{
// Skip on macOS: https://github.com/SixLabors/ImageSharp/issues/1887
return;
}
if (TestEnvironment.OSArchitecture == Architecture.Arm64)
{
// Skip on ARM64: https://github.com/SixLabors/ImageSharp/issues/2342
return;
}
if (!TestEnvironment.RunsOnCI)
{
// This may fail in local runs resulting in high memory load.
// Remove the condition for local debugging!
return;
}
// RunTest(length.ToString());
RemoteExecutor.Invoke(RunTest, length.ToString()).Dispose();
RemoteExecutor.Invoke(RunTest, length.ToString(CultureInfo.InvariantCulture)).Dispose();
static void RunTest(string lengthStr)
{
UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(512, 1024, 16 * 1024, 1024);
int lengthInner = int.Parse(lengthStr);
int lengthInner = int.Parse(lengthStr, CultureInfo.InvariantCulture);
// This test verifies pooling behavior when an `IMemoryOwner<byte>` is leaked (not disposed)
// and must be returned to the pool by finalization.
//
// We do NOT use a sentinel byte value to prove reuse because the contents of pooled buffers
// are not required to be preserved across rentals.
//
// Instead, we assert that after forcing GC+finalization, renting the same size again does not
// increase `UnmanagedMemoryHandle.TotalOutstandingHandles` above a baseline.
// Establish a baseline: allocate+dispose once so the pool has a chance to materialize/retain buffers.
allocator.Allocate<byte>(lengthInner).Dispose();
int baselineHandles = UnmanagedMemoryHandle.TotalOutstandingHandles;
// Leak one allocation and force finalization.
AllocateSingleAndForget(allocator, lengthInner);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
AllocateSingleAndForget(allocator, lengthInner, true);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
// Allocate again. If the leaked owner was finalized correctly and returned to the pool,
// this should not require additional unmanaged allocations (ie, the handle count must not grow).
allocator.Allocate<byte>(lengthInner).Dispose();
using IMemoryOwner<byte> g = allocator.Allocate<byte>(lengthInner);
Assert.Equal(42, g.GetSpan()[0]);
GC.KeepAlive(allocator);
// Note: we use "<=" rather than "==". The pool may legitimately trim and free retained buffers,
// reducing the handle count between baseline and check. The invariant is "no growth".
Assert.True(UnmanagedMemoryHandle.TotalOutstandingHandles <= baselineHandles);
}
}
[MethodImpl(MethodImplOptions.NoInlining)]
private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length, bool check = false)
private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length)
{
IMemoryOwner<byte> g = allocator.Allocate<byte>(length);
if (check)
{
Assert.Equal(42, g.GetSpan()[0]);
}
// Allocate and intentionally do not dispose.
IMemoryOwner<byte>? g = allocator.Allocate<byte>(length);
// Touch the memory to ensure the buffer is actually materialized/usable.
g.GetSpan()[0] = 42;
if (length < 512)
{
// For ArrayPool.Shared, first array will be returned to the TLS storage of the finalizer thread,
// repeat rental to make sure per-core buckets are also utilized.
// For ArrayPool.Shared, the first rented array may be stored in TLS on the finalizer thread.
// Repeat rental to increase the chance that per-core buckets are involved when length
// is small and allocations go through ArrayPool.
IMemoryOwner<byte> g1 = allocator.Allocate<byte>(length);
g1.GetSpan()[0] = 42;
g1 = null;
}
g = null;
}
[Fact]

Write Preview
Loading…
Cancel
Save