From fd4e3a51081309dae438e20f3ebbbdb56ee6f5be Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 25 Feb 2022 00:44:06 +1100 Subject: [PATCH] Scale allocated chunk sizes to match allocations --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 68 ++++++++++++------ .../IO/ChunkedMemoryStreamTests.cs | 70 ++++++++----------- 2 files changed, 77 insertions(+), 61 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index e39930def..9c242b558 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -17,9 +17,19 @@ namespace SixLabors.ImageSharp.IO internal sealed class ChunkedMemoryStream : Stream { /// - /// The default length in bytes of each buffer chunk. + /// The default length in bytes of each buffer chunk when allocating large buffers. /// - public const int DefaultBufferLength = 128 * 1024; + public const int DefaultLargeChunkSize = 1024 * 1024 * 4; // 4 Mb + + /// + /// The threshold at which to switch to using the large buffer. + /// + public const int DefaultLargeChunkThreshold = DefaultLargeChunkSize / 4; // 1 Mb + + /// + /// The default length in bytes of each buffer chunk when allocating small buffers. + /// + public const int DefaultSmallChunkSize = DefaultLargeChunkSize / 32; // 128 Kb // The memory allocator. private readonly MemoryAllocator allocator; @@ -27,8 +37,18 @@ namespace SixLabors.ImageSharp.IO // Data private MemoryChunk memoryChunk; - // The length of each buffer chunk - private readonly int chunkLength; + // The length, in bytes, of each large buffer chunk + private readonly int largeChunkSize; + + // The length, in bytes of the total allocation threshold that triggers switching from + // small to large buffer chunks. + private readonly int largeChunkThreshold; + + // The current length, in bytes, of each buffer chunk + private int chunkSize; + + // The total allocation length, in bytes + private int totalAllocation; // Has the stream been disposed. private bool isDisposed; @@ -49,21 +69,15 @@ namespace SixLabors.ImageSharp.IO /// Initializes a new instance of the class. /// public ChunkedMemoryStream(MemoryAllocator allocator) - : this(DefaultBufferLength, allocator) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The length, in bytes of each buffer chunk. - /// The memory allocator. - public ChunkedMemoryStream(int bufferLength, MemoryAllocator allocator) { - Guard.MustBeGreaterThan(bufferLength, 0, nameof(bufferLength)); Guard.NotNull(allocator, nameof(allocator)); - this.chunkLength = bufferLength; + // Tweak our buffer sizes to take the minimum of the provided buffer sizes + // or values scaled from the allocator buffer capacity which provides us with the largest + // available contiguous buffer size. + this.largeChunkSize = Math.Min(DefaultLargeChunkSize, allocator.GetBufferCapacityInBytes()); + this.largeChunkThreshold = Math.Min(DefaultLargeChunkThreshold, this.largeChunkSize / 4); + this.chunkSize = Math.Min(DefaultSmallChunkSize, this.largeChunkSize / 32); this.allocator = allocator; } @@ -191,6 +205,9 @@ namespace SixLabors.ImageSharp.IO case SeekOrigin.End: this.Position = this.Length + offset; break; + default: + ThrowInvalidSeek(); + break; } return this.Position; @@ -219,6 +236,7 @@ namespace SixLabors.ImageSharp.IO this.memoryChunk = null; this.writeChunk = null; this.readChunk = null; + this.totalAllocation = 0; } finally { @@ -519,17 +537,25 @@ namespace SixLabors.ImageSharp.IO } [MethodImpl(MethodImplOptions.NoInlining)] - private static void ThrowDisposed() - => throw new ObjectDisposedException(null, "The stream is closed."); + private static void ThrowDisposed() => throw new ObjectDisposedException(null, "The stream is closed."); [MethodImpl(MethodImplOptions.NoInlining)] - private static void ThrowArgumentOutOfRange(string value) - => throw new ArgumentOutOfRangeException(value); + private static void ThrowArgumentOutOfRange(string value) => throw new ArgumentOutOfRangeException(value); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowInvalidSeek() => throw new ArgumentException("Invalid seek origin."); [MethodImpl(MethodImplOptions.AggressiveInlining)] private MemoryChunk AllocateMemoryChunk() { - IMemoryOwner buffer = this.allocator.Allocate(this.chunkLength); + if (this.totalAllocation >= this.largeChunkThreshold) + { + this.chunkSize = this.largeChunkSize; + } + + IMemoryOwner buffer = this.allocator.Allocate(this.chunkSize); + this.totalAllocation += this.chunkSize; + return new MemoryChunk { Buffer = buffer, diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 00a178c8f..455730e71 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -20,17 +20,7 @@ namespace SixLabors.ImageSharp.Tests.IO { private readonly MemoryAllocator allocator; - public ChunkedMemoryStreamTests() - { - this.allocator = Configuration.Default.MemoryAllocator; - } - - [Fact] - public void MemoryStream_Ctor_InvalidCapacities() - { - Assert.Throws(() => new ChunkedMemoryStream(int.MinValue, this.allocator)); - Assert.Throws(() => new ChunkedMemoryStream(0, this.allocator)); - } + public ChunkedMemoryStreamTests() => this.allocator = Configuration.Default.MemoryAllocator; [Fact] public void MemoryStream_GetPositionTest_Negative() @@ -61,11 +51,11 @@ namespace SixLabors.ImageSharp.Tests.IO } [Theory] - [InlineData(ChunkedMemoryStream.DefaultBufferLength)] - [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 1.5))] - [InlineData(ChunkedMemoryStream.DefaultBufferLength * 4)] - [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 5.5))] - [InlineData(ChunkedMemoryStream.DefaultBufferLength * 8)] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize)] + [InlineData((int)(ChunkedMemoryStream.DefaultSmallChunkSize * 1.5))] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize * 4)] + [InlineData((int)(ChunkedMemoryStream.DefaultSmallChunkSize * 5.5))] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize * 16)] public void MemoryStream_ReadByteTest(int length) { using MemoryStream ms = this.CreateTestStream(length); @@ -73,7 +63,7 @@ namespace SixLabors.ImageSharp.Tests.IO ms.CopyTo(cms); cms.Position = 0; - var expected = ms.ToArray(); + byte[] expected = ms.ToArray(); for (int i = 0; i < expected.Length; i++) { @@ -82,11 +72,11 @@ namespace SixLabors.ImageSharp.Tests.IO } [Theory] - [InlineData(ChunkedMemoryStream.DefaultBufferLength)] - [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 1.5))] - [InlineData(ChunkedMemoryStream.DefaultBufferLength * 4)] - [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 5.5))] - [InlineData(ChunkedMemoryStream.DefaultBufferLength * 8)] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize)] + [InlineData((int)(ChunkedMemoryStream.DefaultSmallChunkSize * 1.5))] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize * 4)] + [InlineData((int)(ChunkedMemoryStream.DefaultSmallChunkSize * 5.5))] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize * 16)] public void MemoryStream_ReadByteBufferTest(int length) { using MemoryStream ms = this.CreateTestStream(length); @@ -94,8 +84,8 @@ namespace SixLabors.ImageSharp.Tests.IO ms.CopyTo(cms); cms.Position = 0; - var expected = ms.ToArray(); - var buffer = new byte[2]; + byte[] expected = ms.ToArray(); + byte[] buffer = new byte[2]; for (int i = 0; i < expected.Length; i += 2) { cms.Read(buffer); @@ -105,11 +95,11 @@ namespace SixLabors.ImageSharp.Tests.IO } [Theory] - [InlineData(ChunkedMemoryStream.DefaultBufferLength)] - [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 1.5))] - [InlineData(ChunkedMemoryStream.DefaultBufferLength * 4)] - [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 5.5))] - [InlineData(ChunkedMemoryStream.DefaultBufferLength * 8)] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize)] + [InlineData((int)(ChunkedMemoryStream.DefaultSmallChunkSize * 1.5))] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize * 4)] + [InlineData((int)(ChunkedMemoryStream.DefaultSmallChunkSize * 5.5))] + [InlineData(ChunkedMemoryStream.DefaultSmallChunkSize * 16)] public void MemoryStream_ReadByteBufferSpanTest(int length) { using MemoryStream ms = this.CreateTestStream(length); @@ -117,7 +107,7 @@ namespace SixLabors.ImageSharp.Tests.IO ms.CopyTo(cms); cms.Position = 0; - var expected = ms.ToArray(); + byte[] expected = ms.ToArray(); Span buffer = new byte[2]; for (int i = 0; i < expected.Length; i += 2) { @@ -257,24 +247,24 @@ namespace SixLabors.ImageSharp.Tests.IO public void MemoryStream_CopyTo_Invalid() { ChunkedMemoryStream memoryStream; - const string BufferSize = "bufferSize"; + const string bufferSize = nameof(bufferSize); using (memoryStream = new ChunkedMemoryStream(this.allocator)) { - const string Destination = "destination"; - Assert.Throws(Destination, () => memoryStream.CopyTo(destination: null)); + const string destination = nameof(destination); + Assert.Throws(destination, () => memoryStream.CopyTo(destination: null)); // Validate the destination parameter first. - Assert.Throws(Destination, () => memoryStream.CopyTo(destination: null, bufferSize: 0)); - Assert.Throws(Destination, () => memoryStream.CopyTo(destination: null, bufferSize: -1)); + Assert.Throws(destination, () => memoryStream.CopyTo(destination: null, bufferSize: 0)); + Assert.Throws(destination, () => memoryStream.CopyTo(destination: null, bufferSize: -1)); // Then bufferSize. - Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: 0)); // 0-length buffer doesn't make sense. - Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: -1)); + Assert.Throws(bufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: 0)); // 0-length buffer doesn't make sense. + Assert.Throws(bufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: -1)); } // After the Stream is disposed, we should fail on all CopyTos. - Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: 0)); // Not before bufferSize is validated. - Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: -1)); + Assert.Throws(bufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: 0)); // Not before bufferSize is validated. + Assert.Throws(bufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: -1)); ChunkedMemoryStream disposedStream = memoryStream; @@ -369,7 +359,7 @@ namespace SixLabors.ImageSharp.Tests.IO private MemoryStream CreateTestStream(int length) { - var buffer = new byte[length]; + byte[] buffer = new byte[length]; var random = new Random(); random.NextBytes(buffer);