From 706babe2a2caa07185c86a16ffe17fc6d51afa78 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sun, 22 Jul 2018 17:01:39 +0200 Subject: [PATCH] introducing BufferManager --- ImageSharp.sln.DotSettings | 5 +- src/ImageSharp/Memory/BufferManager.cs | 73 ++++++++ src/ImageSharp/Memory/IBuffer{T}.cs | 8 +- .../Memory/MemoryAllocatorExtensions.cs | 11 +- .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 50 ++---- .../Memory/BufferManagerTests.cs | 158 ++++++++++++++++++ .../Processors/Transforms/ResizeTests.cs | 3 +- .../TestUtilities/TestMemoryAllocator.cs | 55 ++++++ .../TestUtilities/TestMemoryManager.cs | 19 +-- .../TestUtilities/TestUtils.cs | 2 +- 10 files changed, 328 insertions(+), 56 deletions(-) create mode 100644 src/ImageSharp/Memory/BufferManager.cs create mode 100644 tests/ImageSharp.Tests/Memory/BufferManagerTests.cs create mode 100644 tests/ImageSharp.Tests/TestUtilities/TestMemoryAllocator.cs diff --git a/ImageSharp.sln.DotSettings b/ImageSharp.sln.DotSettings index 435aad73b..432f4524a 100644 --- a/ImageSharp.sln.DotSettings +++ b/ImageSharp.sln.DotSettings @@ -343,8 +343,11 @@ <Entry DisplayName="All other members" /> </TypePattern> </Patterns> - True + False True + // Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + AC DC DCT diff --git a/src/ImageSharp/Memory/BufferManager.cs b/src/ImageSharp/Memory/BufferManager.cs new file mode 100644 index 000000000..f1cbb7512 --- /dev/null +++ b/src/ImageSharp/Memory/BufferManager.cs @@ -0,0 +1,73 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Buffers; + +namespace SixLabors.Memory +{ + /// + /// Holds a that is either OWNED or CONSUMED. + /// Implements content transfer logic in that depends on the ownership status. + /// This is needed to transfer the contents of a temporary to a persistent + /// + internal struct BufferManager : IDisposable + { + public BufferManager(IMemoryOwner memoryOwner) + { + this.MemoryOwner = memoryOwner; + this.Memory = memoryOwner.Memory; + } + + public BufferManager(Memory memory) + { + this.Memory = memory; + this.MemoryOwner = null; + } + + public IMemoryOwner MemoryOwner { get; private set; } + + public Memory Memory { get; private set; } + + public bool OwnsMemory => this.MemoryOwner != null; + + /// + /// Swaps the contents of 'destination' with 'source' if the buffers are owned (1), + /// copies the contents of 'source' to 'destination' otherwise (2). Buffers should be of same size in case 2! + /// + public static void SwapOrCopyContent(ref BufferManager destination, ref BufferManager source) + { + if (source.OwnsMemory && destination.OwnsMemory) + { + SwapContents(ref destination, ref source); + } + else + { + if (destination.Memory.Length != source.Memory.Length) + { + throw new InvalidOperationException("SwapOrCopyContents(): buffers should both owned or the same size!"); + } + + source.Memory.CopyTo(destination.Memory); + } + } + + /// + public void Dispose() + { + this.MemoryOwner?.Dispose(); + } + + private static void SwapContents(ref BufferManager a, ref BufferManager b) + { + IMemoryOwner tempOwner = a.MemoryOwner; + Memory tempMemory = a.Memory; + + a.MemoryOwner = b.MemoryOwner; + a.Memory = b.Memory; + + b.MemoryOwner = tempOwner; + b.Memory = tempMemory; + } + } +} \ No newline at end of file diff --git a/src/ImageSharp/Memory/IBuffer{T}.cs b/src/ImageSharp/Memory/IBuffer{T}.cs index 847f2741d..73406971a 100644 --- a/src/ImageSharp/Memory/IBuffer{T}.cs +++ b/src/ImageSharp/Memory/IBuffer{T}.cs @@ -19,14 +19,14 @@ namespace SixLabors.Memory where T : struct { /// - /// Gets the ownerd/consumed by this buffer. + /// Gets a value indicating whether this instance is owning the . /// - Memory Memory { get; } + bool IsMemoryOwner { get; } /// - /// Gets a value indicating whether this instance is owning the . + /// Gets the ownerd/consumed by this buffer. /// - bool IsMemoryOwner { get; } + Memory Memory { get; } /// /// Gets the span to the memory "promised" by this buffer when it's OWNED (1). diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index 2f296636d..899d2f0f6 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -7,7 +7,11 @@ namespace SixLabors.Memory /// internal static class MemoryAllocatorExtensions { - public static Buffer2D Allocate2D(this MemoryAllocator memoryAllocator, int width, int height, AllocationOptions options = AllocationOptions.None) + public static Buffer2D Allocate2D( + this MemoryAllocator memoryAllocator, + int width, + int height, + AllocationOptions options = AllocationOptions.None) where T : struct { IBuffer buffer = memoryAllocator.Allocate(width * height, options); @@ -15,7 +19,10 @@ namespace SixLabors.Memory return new Buffer2D(buffer, width, height); } - public static Buffer2D Allocate2D(this MemoryAllocator memoryAllocator, Size size, AllocationOptions options = AllocationOptions.None) + public static Buffer2D Allocate2D( + this MemoryAllocator memoryAllocator, + Size size, + AllocationOptions options = AllocationOptions.None) where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, options); diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 5dd8572dc..af9c8bd5b 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -1,20 +1,20 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -// ReSharper disable InconsistentNaming -namespace SixLabors.ImageSharp.Tests.Memory -{ - using System; - using System.Runtime.CompilerServices; - using System.Runtime.InteropServices; +using System; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; - using SixLabors.ImageSharp.PixelFormats; - using SixLabors.Memory; - using SixLabors.ImageSharp.Tests.Common; - using SixLabors.Primitives; +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.Memory; +using SixLabors.Primitives; - using Xunit; +using Xunit; +// ReSharper disable InconsistentNaming + +namespace SixLabors.ImageSharp.Tests.Memory +{ public class Buffer2DTests { // ReSharper disable once ClassNeverInstantiated.Local @@ -30,31 +30,7 @@ namespace SixLabors.ImageSharp.Tests.Memory } } - private MemoryAllocator MemoryAllocator { get; } = new MockMemoryAllocator(); - - private class MockMemoryAllocator : MemoryAllocator - { - internal override IBuffer Allocate(int length, AllocationOptions options) - { - var array = new T[length + 42]; - - if (options == AllocationOptions.None) - { - Span data = MemoryMarshal.Cast(array.AsSpan()); - for (int i = 0; i < data.Length; i++) - { - data[i] = 42; - } - } - - return new BasicArrayBuffer(array, length); - } - - internal override IManagedByteBuffer AllocateManagedByteBuffer(int length, AllocationOptions options) - { - throw new NotImplementedException(); - } - } + private MemoryAllocator MemoryAllocator { get; } = new TestMemoryAllocator(); [Theory] [InlineData(7, 42)] @@ -134,7 +110,7 @@ namespace SixLabors.ImageSharp.Tests.Memory public class SwapOrCopyContent { - private MemoryAllocator MemoryAllocator { get; } = new MockMemoryAllocator(); + private MemoryAllocator MemoryAllocator { get; } = new TestMemoryAllocator(); [Fact] public void WhenBothBuffersAreMemoryOwners_ShouldSwap() diff --git a/tests/ImageSharp.Tests/Memory/BufferManagerTests.cs b/tests/ImageSharp.Tests/Memory/BufferManagerTests.cs new file mode 100644 index 000000000..d08e734e8 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/BufferManagerTests.cs @@ -0,0 +1,158 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Buffers; + +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.Memory; + +using Xunit; +// ReSharper disable InconsistentNaming + +namespace SixLabors.ImageSharp.Tests.Memory +{ + public class BufferManagerTests + { + public class Construction + { + [Fact] + public void InitializeAsOwner_MemoryOwner_IsPresent() + { + var data = new Rgba32[21]; + var mmg = new TestMemoryManager(data); + + var a = new BufferManager(mmg); + + Assert.Equal(mmg, a.MemoryOwner); + Assert.Equal(mmg.Memory, a.Memory); + Assert.True(a.OwnsMemory); + } + + [Fact] + public void InitializeAsObserver_MemoryOwner_IsNull() + { + var data = new Rgba32[21]; + var mmg = new TestMemoryManager(data); + + var a = new BufferManager(mmg.Memory); + + Assert.Null(a.MemoryOwner); + Assert.Equal(mmg.Memory, a.Memory); + Assert.False(a.OwnsMemory); + } + } + + public class Dispose + { + [Fact] + public void WhenOwnershipIsTransfered_ShouldDisposeMemoryOwner() + { + var mmg = new TestMemoryManager(new int[10]); + var bmg = new BufferManager(mmg); + + bmg.Dispose(); + Assert.True(mmg.IsDisposed); + } + + [Fact] + public void WhenMemoryObserver_ShouldNotDisposeAnything() + { + var mmg = new TestMemoryManager(new int[10]); + var bmg = new BufferManager(mmg.Memory); + + bmg.Dispose(); + Assert.False(mmg.IsDisposed); + } + } + + public class SwapOrCopyContent + { + private MemoryAllocator MemoryAllocator { get; } = new TestMemoryAllocator(); + + private BufferManager AllocateBufferManager(int length, AllocationOptions options = AllocationOptions.None) + where T : struct + { + var owner = (IMemoryOwner)this.MemoryAllocator.Allocate(length, options); + return new BufferManager(owner); + } + + [Fact] + public void WhenBothAreMemoryOwners_ShouldSwap() + { + BufferManager a = this.AllocateBufferManager(13); + BufferManager b = this.AllocateBufferManager(17); + + IMemoryOwner aa = a.MemoryOwner; + IMemoryOwner bb = b.MemoryOwner; + + Memory aaa = a.Memory; + Memory bbb = b.Memory; + + BufferManager.SwapOrCopyContent(ref a, ref b); + + Assert.Equal(bb, a.MemoryOwner); + Assert.Equal(aa, b.MemoryOwner); + + Assert.Equal(bbb, a.Memory); + Assert.Equal(aaa, b.Memory); + Assert.NotEqual(a.Memory, b.Memory); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void WhenDestIsNotMemoryOwner_SameSize_ShouldCopy(bool sourceIsOwner) + { + var data = new Rgba32[21]; + var color = new Rgba32(1, 2, 3, 4); + + var destOwner = new TestMemoryManager(data); + var dest = new BufferManager(destOwner.Memory); + + var sourceOwner = (IMemoryOwner)this.MemoryAllocator.Allocate(21); + + BufferManager source = sourceIsOwner + ? new BufferManager(sourceOwner) + : new BufferManager(sourceOwner.Memory); + + sourceOwner.Memory.Span[10] = color; + + // Act: + BufferManager.SwapOrCopyContent(ref dest, ref source); + + // Assert: + Assert.Equal(color, dest.Memory.Span[10]); + Assert.NotEqual(sourceOwner, dest.MemoryOwner); + Assert.NotEqual(destOwner, source.MemoryOwner); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void WhenDestIsNotMemoryOwner_DifferentSize_Throws(bool sourceIsOwner) + { + var data = new Rgba32[21]; + var color = new Rgba32(1, 2, 3, 4); + + var destOwner = new TestMemoryManager(data); + var dest = new BufferManager(destOwner.Memory); + + var sourceOwner = (IMemoryOwner)this.MemoryAllocator.Allocate(22); + + BufferManager source = sourceIsOwner + ? new BufferManager(sourceOwner) + : new BufferManager(sourceOwner.Memory); + sourceOwner.Memory.Span[10] = color; + + // Act: + Assert.ThrowsAny( + () => BufferManager.SwapOrCopyContent(ref dest, ref source) + ); + + Assert.Equal(color, source.Memory.Span[10]); + Assert.NotEqual(color, dest.Memory.Span[10]); + } + } + } +} \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs index c7efbb1e0..746d8da16 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs @@ -3,6 +3,7 @@ using System; +using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Transforms; @@ -91,7 +92,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { using (Image image0 = provider.GetImage()) { - var mmg = TestMemoryManager.CreateAsCopyOfPixelData(image0); + var mmg = TestMemoryManager.CreateAsCopyOf(image0.GetPixelSpan()); using (var image1 = Image.WrapMemory(mmg.Memory, image0.Width, image0.Height)) { diff --git a/tests/ImageSharp.Tests/TestUtilities/TestMemoryAllocator.cs b/tests/ImageSharp.Tests/TestUtilities/TestMemoryAllocator.cs new file mode 100644 index 000000000..7993b3e99 --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/TestMemoryAllocator.cs @@ -0,0 +1,55 @@ +using System; +using System.Runtime.InteropServices; + +using SixLabors.Memory; + +namespace SixLabors.ImageSharp.Tests.Memory +{ + internal class TestMemoryAllocator : MemoryAllocator + { + public TestMemoryAllocator(byte dirtyValue = 42) + { + this.DirtyValue = dirtyValue; + } + + /// + /// The value to initilazie the result buffer with, with non-clean options () + /// + public byte DirtyValue { get; } + + internal override IBuffer Allocate(int length, AllocationOptions options = AllocationOptions.None) + { + T[] array = this.AllocateArray(length, options); + + return new BasicArrayBuffer(array, length); + } + + internal override IManagedByteBuffer AllocateManagedByteBuffer(int length, AllocationOptions options = AllocationOptions.None) + { + byte[] array = this.AllocateArray(length, options); + return new ManagedByteBuffer(array); + } + + private T[] AllocateArray(int length, AllocationOptions options) + where T : struct + { + var array = new T[length + 42]; + + if (options == AllocationOptions.None) + { + Span data = MemoryMarshal.Cast(array.AsSpan()); + data.Fill(this.DirtyValue); + } + + return array; + } + + private class ManagedByteBuffer : BasicArrayBuffer, IManagedByteBuffer + { + public ManagedByteBuffer(byte[] array) + : base(array) + { + } + } + } +} \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestUtilities/TestMemoryManager.cs b/tests/ImageSharp.Tests/TestUtilities/TestMemoryManager.cs index e7ecb2dda..ce3cfbc9e 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestMemoryManager.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestMemoryManager.cs @@ -7,18 +7,16 @@ namespace SixLabors.ImageSharp.Tests using SixLabors.ImageSharp.PixelFormats; class TestMemoryManager : MemoryManager - where T : struct, IPixel + where T : struct { public TestMemoryManager(T[] pixelArray) { this.PixelArray = pixelArray; } - public T[] PixelArray { get; } + public T[] PixelArray { get; private set; } - protected override void Dispose(bool disposing) - { - } + public bool IsDisposed { get; private set; } public override Span GetSpan() { @@ -35,16 +33,17 @@ namespace SixLabors.ImageSharp.Tests throw new NotImplementedException(); } - public static TestMemoryManager CreateAsCopyOfPixelData(Span pixelData) + public static TestMemoryManager CreateAsCopyOf(Span copyThisBuffer) { - var pixelArray = new T[pixelData.Length]; - pixelData.CopyTo(pixelArray); + var pixelArray = new T[copyThisBuffer.Length]; + copyThisBuffer.CopyTo(pixelArray); return new TestMemoryManager(pixelArray); } - public static TestMemoryManager CreateAsCopyOfPixelData(Image image) + protected override void Dispose(bool disposing) { - return CreateAsCopyOfPixelData(image.GetPixelSpan()); + this.IsDisposed = true; + this.PixelArray = null; } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs b/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs index 03ab422e5..a6ea76f2d 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs @@ -214,7 +214,7 @@ namespace SixLabors.ImageSharp.Tests using (Image image0 = provider.GetImage()) { - var mmg = TestMemoryManager.CreateAsCopyOfPixelData(image0.GetPixelSpan()); + var mmg = TestMemoryManager.CreateAsCopyOf(image0.GetPixelSpan()); using (var image1 = Image.WrapMemory(mmg.Memory, image0.Width, image0.Height)) {