Browse Source

Feedback updates and massively expand write tests

pull/2828/head
James Jackson-South 2 years ago
parent
commit
630166211c
  1. 148
      src/ImageSharp/IO/ChunkedMemoryStream.cs
  2. 88
      tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs

148
src/ImageSharp/IO/ChunkedMemoryStream.cs

@ -2,7 +2,6 @@
// Licensed under the Six Labors Split License. // Licensed under the Six Labors Split License.
using System.Buffers; using System.Buffers;
using System.Collections;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Runtime.InteropServices; using System.Runtime.InteropServices;
using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Memory;
@ -14,15 +13,13 @@ namespace SixLabors.ImageSharp.IO;
/// Chunks are allocated by the <see cref="MemoryAllocator"/> assigned via the constructor /// Chunks are allocated by the <see cref="MemoryAllocator"/> assigned via the constructor
/// and is designed to take advantage of buffer pooling when available. /// and is designed to take advantage of buffer pooling when available.
/// </summary> /// </summary>
public class ChunkedMemoryStream : Stream internal sealed class ChunkedMemoryStream : Stream
{ {
private readonly MemoryChunkBuffer memoryChunkBuffer; private readonly MemoryChunkBuffer memoryChunkBuffer;
private readonly byte[] singleByteBuffer = new byte[1];
private long length; private long length;
private long position; private long position;
private int currentChunk; private int bufferIndex;
private int currentChunkIndex; private int chunkIndex;
private bool isDisposed; private bool isDisposed;
/// <summary> /// <summary>
@ -95,21 +92,13 @@ public class ChunkedMemoryStream : Stream
/// <inheritdoc/> /// <inheritdoc/>
public override int ReadByte() public override int ReadByte()
{ {
this.EnsureNotDisposed(); Unsafe.SkipInit(out byte b);
if (this.position >= this.length) return this.Read(MemoryMarshal.CreateSpan(ref b, 1)) == 1 ? b : -1;
{
return -1;
}
_ = this.Read(this.singleByteBuffer, 0, 1);
return MemoryMarshal.GetReference<byte>(this.singleByteBuffer);
} }
/// <inheritdoc/> /// <inheritdoc/>
public override int Read(byte[] buffer, int offset, int count) public override int Read(byte[] buffer, int offset, int count)
{ {
this.EnsureNotDisposed();
Guard.NotNull(buffer, nameof(buffer)); Guard.NotNull(buffer, nameof(buffer));
Guard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); Guard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset));
Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count));
@ -135,19 +124,14 @@ public class ChunkedMemoryStream : Stream
return 0; return 0;
} }
if (remaining > count) int bytesToRead = count;
{
remaining = count;
}
int bytesToRead = (int)remaining;
int bytesRead = 0; int bytesRead = 0;
while (bytesToRead != 0 && this.currentChunk != this.memoryChunkBuffer.Length) while (bytesToRead > 0 && this.bufferIndex != this.memoryChunkBuffer.Length)
{ {
bool moveToNextChunk = false; bool moveToNextChunk = false;
MemoryChunk chunk = this.memoryChunkBuffer[this.currentChunk]; MemoryChunk chunk = this.memoryChunkBuffer[this.bufferIndex];
int n = bytesToRead; int n = bytesToRead;
int remainingBytesInCurrentChunk = chunk.Length - this.currentChunkIndex; int remainingBytesInCurrentChunk = chunk.Length - this.chunkIndex;
if (n >= remainingBytesInCurrentChunk) if (n >= remainingBytesInCurrentChunk)
{ {
n = remainingBytesInCurrentChunk; n = remainingBytesInCurrentChunk;
@ -155,19 +139,19 @@ public class ChunkedMemoryStream : Stream
} }
// Read n bytes from the current chunk // Read n bytes from the current chunk
chunk.Buffer.Memory.Span.Slice(this.currentChunkIndex, n).CopyTo(buffer.Slice(offset, n)); chunk.Buffer.Memory.Span.Slice(this.chunkIndex, n).CopyTo(buffer.Slice(offset, n));
bytesToRead -= n; bytesToRead -= n;
offset += n; offset += n;
bytesRead += n; bytesRead += n;
if (moveToNextChunk) if (moveToNextChunk)
{ {
this.currentChunkIndex = 0; this.chunkIndex = 0;
this.currentChunk++; this.bufferIndex++;
} }
else else
{ {
this.currentChunkIndex += n; this.chunkIndex += n;
} }
} }
@ -177,11 +161,7 @@ public class ChunkedMemoryStream : Stream
/// <inheritdoc/> /// <inheritdoc/>
public override void WriteByte(byte value) public override void WriteByte(byte value)
{ => this.Write(MemoryMarshal.CreateSpan(ref value, 1));
this.EnsureNotDisposed();
MemoryMarshal.Write(this.singleByteBuffer, ref value);
this.Write(this.singleByteBuffer, 0, 1);
}
/// <inheritdoc/> /// <inheritdoc/>
public override void Write(byte[] buffer, int offset, int count) public override void Write(byte[] buffer, int offset, int count)
@ -213,19 +193,14 @@ public class ChunkedMemoryStream : Stream
remaining = this.memoryChunkBuffer.Length - this.position; remaining = this.memoryChunkBuffer.Length - this.position;
} }
if (remaining > count) int bytesToWrite = count;
{
remaining = count;
}
int bytesToWrite = (int)remaining;
int bytesWritten = 0; int bytesWritten = 0;
while (bytesToWrite != 0 && this.currentChunk != this.memoryChunkBuffer.Length) while (bytesToWrite > 0 && this.bufferIndex != this.memoryChunkBuffer.Length)
{ {
bool moveToNextChunk = false; bool moveToNextChunk = false;
MemoryChunk chunk = this.memoryChunkBuffer[this.currentChunk]; MemoryChunk chunk = this.memoryChunkBuffer[this.bufferIndex];
int n = bytesToWrite; int n = bytesToWrite;
int remainingBytesInCurrentChunk = chunk.Length - this.currentChunkIndex; int remainingBytesInCurrentChunk = chunk.Length - this.chunkIndex;
if (n >= remainingBytesInCurrentChunk) if (n >= remainingBytesInCurrentChunk)
{ {
n = remainingBytesInCurrentChunk; n = remainingBytesInCurrentChunk;
@ -233,19 +208,19 @@ public class ChunkedMemoryStream : Stream
} }
// Write n bytes to the current chunk // Write n bytes to the current chunk
buffer.Slice(offset, n).CopyTo(chunk.Buffer.Slice(this.currentChunkIndex, n)); buffer.Slice(offset, n).CopyTo(chunk.Buffer.Slice(this.chunkIndex, n));
bytesToWrite -= n; bytesToWrite -= n;
offset += n; offset += n;
bytesWritten += n; bytesWritten += n;
if (moveToNextChunk) if (moveToNextChunk)
{ {
this.currentChunkIndex = 0; this.chunkIndex = 0;
this.currentChunk++; this.bufferIndex++;
} }
else else
{ {
this.currentChunkIndex += n; this.chunkIndex += n;
} }
} }
@ -275,12 +250,12 @@ public class ChunkedMemoryStream : Stream
int bytesToRead = (int)remaining; int bytesToRead = (int)remaining;
int bytesRead = 0; int bytesRead = 0;
while (bytesToRead != 0 && this.currentChunk != this.memoryChunkBuffer.Length) while (bytesToRead > 0 && this.bufferIndex != this.memoryChunkBuffer.Length)
{ {
bool moveToNextChunk = false; bool moveToNextChunk = false;
MemoryChunk chunk = this.memoryChunkBuffer[this.currentChunk]; MemoryChunk chunk = this.memoryChunkBuffer[this.bufferIndex];
int n = bytesToRead; int n = bytesToRead;
int remainingBytesInCurrentChunk = chunk.Length - this.currentChunkIndex; int remainingBytesInCurrentChunk = chunk.Length - this.chunkIndex;
if (n >= remainingBytesInCurrentChunk) if (n >= remainingBytesInCurrentChunk)
{ {
n = remainingBytesInCurrentChunk; n = remainingBytesInCurrentChunk;
@ -288,18 +263,18 @@ public class ChunkedMemoryStream : Stream
} }
// Read n bytes from the current chunk // Read n bytes from the current chunk
stream.Write(chunk.Buffer.Memory.Span.Slice(this.currentChunkIndex, n)); stream.Write(chunk.Buffer.Memory.Span.Slice(this.chunkIndex, n));
bytesToRead -= n; bytesToRead -= n;
bytesRead += n; bytesRead += n;
if (moveToNextChunk) if (moveToNextChunk)
{ {
this.currentChunkIndex = 0; this.chunkIndex = 0;
this.currentChunk++; this.bufferIndex++;
} }
else else
{ {
this.currentChunkIndex += n; this.chunkIndex += n;
} }
} }
@ -338,8 +313,8 @@ public class ChunkedMemoryStream : Stream
this.memoryChunkBuffer.Dispose(); this.memoryChunkBuffer.Dispose();
} }
this.currentChunk = 0; this.bufferIndex = 0;
this.currentChunkIndex = 0; this.chunkIndex = 0;
this.position = 0; this.position = 0;
this.length = 0; this.length = 0;
} }
@ -366,8 +341,8 @@ public class ChunkedMemoryStream : Stream
// If the new position is greater than the length of the stream, set the position to the end of the stream // If the new position is greater than the length of the stream, set the position to the end of the stream
if (offset > 0 && offset >= this.memoryChunkBuffer.Length) if (offset > 0 && offset >= this.memoryChunkBuffer.Length)
{ {
this.currentChunk = this.memoryChunkBuffer.ChunkCount - 1; this.bufferIndex = this.memoryChunkBuffer.ChunkCount - 1;
this.currentChunkIndex = this.memoryChunkBuffer[this.currentChunk].Length - 1; this.chunkIndex = this.memoryChunkBuffer[this.bufferIndex].Length - 1;
return; return;
} }
@ -386,10 +361,10 @@ public class ChunkedMemoryStream : Stream
currentChunkIndex++; currentChunkIndex++;
} }
this.currentChunk = currentChunkIndex; this.bufferIndex = currentChunkIndex;
// Safe to cast here as we know the offset is less than the chunk length. // Safe to cast here as we know the offset is less than the chunk length.
this.currentChunkIndex = (int)offset; this.chunkIndex = (int)offset;
} }
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
@ -404,7 +379,7 @@ public class ChunkedMemoryStream : Stream
[MethodImpl(MethodImplOptions.NoInlining)] [MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowDisposed() => throw new ObjectDisposedException(nameof(ChunkedMemoryStream), "The stream is closed."); private static void ThrowDisposed() => throw new ObjectDisposedException(nameof(ChunkedMemoryStream), "The stream is closed.");
private sealed class MemoryChunkBuffer : IEnumerable<MemoryChunk>, IDisposable private sealed class MemoryChunkBuffer : IDisposable
{ {
private readonly List<MemoryChunk> memoryChunks = new(); private readonly List<MemoryChunk> memoryChunks = new();
private readonly MemoryAllocator allocator; private readonly MemoryAllocator allocator;
@ -439,15 +414,19 @@ public class ChunkedMemoryStream : Stream
public void Dispose() public void Dispose()
{ {
this.Dispose(true); if (!this.isDisposed)
GC.SuppressFinalize(this); {
} foreach (MemoryChunk chunk in this.memoryChunks)
{
chunk.Dispose();
}
public IEnumerator<MemoryChunk> GetEnumerator() this.memoryChunks.Clear();
=> ((IEnumerable<MemoryChunk>)this.memoryChunks).GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() this.Length = 0;
=> ((IEnumerable)this.memoryChunks).GetEnumerator(); this.isDisposed = true;
}
}
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
private static int GetChunkSize(int i) private static int GetChunkSize(int i)
@ -459,25 +438,6 @@ public class ChunkedMemoryStream : Stream
const int b4M = 1 << 22; const int b4M = 1 << 22;
return i < 16 ? b128K * (1 << (int)((uint)i / 4)) : b4M; return i < 16 ? b128K * (1 << (int)((uint)i / 4)) : b4M;
} }
private void Dispose(bool disposing)
{
if (!this.isDisposed)
{
if (disposing)
{
foreach (MemoryChunk chunk in this.memoryChunks)
{
chunk.Dispose();
}
this.memoryChunks.Clear();
}
this.Length = 0;
this.isDisposed = true;
}
}
} }
private sealed class MemoryChunk : IDisposable private sealed class MemoryChunk : IDisposable
@ -490,23 +450,13 @@ public class ChunkedMemoryStream : Stream
public int Length { get; init; } public int Length { get; init; }
private void Dispose(bool disposing) public void Dispose()
{ {
if (!this.isDisposed) if (!this.isDisposed)
{ {
if (disposing) this.Buffer.Dispose();
{
this.Buffer.Dispose();
}
this.isDisposed = true; this.isDisposed = true;
} }
} }
public void Dispose()
{
this.Dispose(disposing: true);
GC.SuppressFinalize(this);
}
} }
} }

88
tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs

@ -13,6 +13,8 @@ namespace SixLabors.ImageSharp.Tests.IO;
/// </summary> /// </summary>
public class ChunkedMemoryStreamTests public class ChunkedMemoryStreamTests
{ {
private readonly Random bufferFiller = new();
/// <summary> /// <summary>
/// The default length in bytes of each buffer chunk when allocating large buffers. /// The default length in bytes of each buffer chunk when allocating large buffers.
/// </summary> /// </summary>
@ -63,7 +65,7 @@ public class ChunkedMemoryStreamTests
[InlineData(DefaultSmallChunkSize * 16)] [InlineData(DefaultSmallChunkSize * 16)]
public void MemoryStream_ReadByteTest(int length) public void MemoryStream_ReadByteTest(int length)
{ {
using MemoryStream ms = CreateTestStream(length); using MemoryStream ms = this.CreateTestStream(length);
using ChunkedMemoryStream cms = new(this.allocator); using ChunkedMemoryStream cms = new(this.allocator);
ms.CopyTo(cms); ms.CopyTo(cms);
@ -84,7 +86,7 @@ public class ChunkedMemoryStreamTests
[InlineData(DefaultSmallChunkSize * 16)] [InlineData(DefaultSmallChunkSize * 16)]
public void MemoryStream_ReadByteBufferTest(int length) public void MemoryStream_ReadByteBufferTest(int length)
{ {
using MemoryStream ms = CreateTestStream(length); using MemoryStream ms = this.CreateTestStream(length);
using ChunkedMemoryStream cms = new(this.allocator); using ChunkedMemoryStream cms = new(this.allocator);
ms.CopyTo(cms); ms.CopyTo(cms);
@ -105,9 +107,10 @@ public class ChunkedMemoryStreamTests
[InlineData(DefaultSmallChunkSize * 4)] [InlineData(DefaultSmallChunkSize * 4)]
[InlineData((int)(DefaultSmallChunkSize * 5.5))] [InlineData((int)(DefaultSmallChunkSize * 5.5))]
[InlineData(DefaultSmallChunkSize * 16)] [InlineData(DefaultSmallChunkSize * 16)]
[InlineData(DefaultSmallChunkSize * 32)]
public void MemoryStream_ReadByteBufferSpanTest(int length) public void MemoryStream_ReadByteBufferSpanTest(int length)
{ {
using MemoryStream ms = CreateTestStream(length); using MemoryStream ms = this.CreateTestStream(length);
using ChunkedMemoryStream cms = new(this.allocator); using ChunkedMemoryStream cms = new(this.allocator);
ms.CopyTo(cms); ms.CopyTo(cms);
@ -122,13 +125,19 @@ public class ChunkedMemoryStreamTests
} }
} }
[Fact] [Theory]
public void MemoryStream_WriteToTests() [InlineData(DefaultSmallChunkSize)]
[InlineData((int)(DefaultSmallChunkSize * 1.5))]
[InlineData(DefaultSmallChunkSize * 4)]
[InlineData((int)(DefaultSmallChunkSize * 5.5))]
[InlineData(DefaultSmallChunkSize * 16)]
[InlineData(DefaultSmallChunkSize * 32)]
public void MemoryStream_WriteToTests(int length)
{ {
using (ChunkedMemoryStream ms2 = new(this.allocator)) using (ChunkedMemoryStream ms2 = new(this.allocator))
{ {
byte[] bytArrRet; byte[] bytArrRet;
byte[] bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; byte[] bytArr = this.CreateTestBuffer(length);
// [] Write to memoryStream, check the memoryStream // [] Write to memoryStream, check the memoryStream
ms2.Write(bytArr, 0, bytArr.Length); ms2.Write(bytArr, 0, bytArr.Length);
@ -150,7 +159,7 @@ public class ChunkedMemoryStreamTests
using (ChunkedMemoryStream ms3 = new(this.allocator)) using (ChunkedMemoryStream ms3 = new(this.allocator))
{ {
byte[] bytArrRet; byte[] bytArrRet;
byte[] bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; byte[] bytArr = this.CreateTestBuffer(length);
ms2.Write(bytArr, 0, bytArr.Length); ms2.Write(bytArr, 0, bytArr.Length);
ms2.WriteTo(ms3); ms2.WriteTo(ms3);
@ -164,13 +173,19 @@ public class ChunkedMemoryStreamTests
} }
} }
[Fact] [Theory]
public void MemoryStream_WriteToSpanTests() [InlineData(DefaultSmallChunkSize)]
[InlineData((int)(DefaultSmallChunkSize * 1.5))]
[InlineData(DefaultSmallChunkSize * 4)]
[InlineData((int)(DefaultSmallChunkSize * 5.5))]
[InlineData(DefaultSmallChunkSize * 16)]
[InlineData(DefaultSmallChunkSize * 32)]
public void MemoryStream_WriteToSpanTests(int length)
{ {
using (ChunkedMemoryStream ms2 = new(this.allocator)) using (ChunkedMemoryStream ms2 = new(this.allocator))
{ {
Span<byte> bytArrRet; Span<byte> bytArrRet;
Span<byte> bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; Span<byte> bytArr = this.CreateTestBuffer(length);
// [] Write to memoryStream, check the memoryStream // [] Write to memoryStream, check the memoryStream
ms2.Write(bytArr, 0, bytArr.Length); ms2.Write(bytArr, 0, bytArr.Length);
@ -194,7 +209,7 @@ public class ChunkedMemoryStreamTests
using (ChunkedMemoryStream ms3 = new(this.allocator)) using (ChunkedMemoryStream ms3 = new(this.allocator))
{ {
Span<byte> bytArrRet; Span<byte> bytArrRet;
Span<byte> bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; Span<byte> bytArr = this.CreateTestBuffer(length);
ms2.Write(bytArr, 0, bytArr.Length); ms2.Write(bytArr, 0, bytArr.Length);
@ -307,7 +322,7 @@ public class ChunkedMemoryStreamTests
return result; return result;
} }
public static IEnumerable<string> AllTestImages = GetAllTestImages(); public static IEnumerable<string> AllTestImages { get; } = GetAllTestImages();
[Theory] [Theory]
[WithFileCollection(nameof(AllTestImages), PixelTypes.Rgba32)] [WithFileCollection(nameof(AllTestImages), PixelTypes.Rgba32)]
@ -337,9 +352,45 @@ public class ChunkedMemoryStreamTests
using FileStream fs = File.OpenRead(fullPath); using FileStream fs = File.OpenRead(fullPath);
using NonSeekableStream nonSeekableStream = new(fs); using NonSeekableStream nonSeekableStream = new(fs);
Image<TPixel> actual = Image.Load<TPixel>(nonSeekableStream); using Image<TPixel> actual = Image.Load<TPixel>(nonSeekableStream);
ImageComparer.Exact.VerifySimilarity(expected, actual);
expected.Dispose();
}
[Theory]
[WithFileCollection(nameof(AllTestImages), PixelTypes.Rgba32)]
public void EncoderIntegrationTest<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
if (!TestEnvironment.Is64BitProcess)
{
return;
}
Image<TPixel> expected;
try
{
expected = provider.GetImage();
}
catch
{
// The image is invalid
return;
}
string fullPath = Path.Combine(
TestEnvironment.InputImagesDirectoryFullPath,
((TestImageProvider<TPixel>.FileProvider)provider).FilePath);
using MemoryStream ms = new();
using NonSeekableStream nonSeekableStream = new(ms);
expected.SaveAsWebp(nonSeekableStream);
using Image<TPixel> actual = Image.Load<TPixel>(nonSeekableStream);
ImageComparer.Exact.VerifySimilarity(expected, actual); ImageComparer.Exact.VerifySimilarity(expected, actual);
expected.Dispose();
} }
public static IEnumerable<object[]> CopyToData() public static IEnumerable<object[]> CopyToData()
@ -363,12 +414,13 @@ public class ChunkedMemoryStreamTests
yield return new object[] { stream3, Array.Empty<byte>() }; yield return new object[] { stream3, Array.Empty<byte>() };
} }
private static MemoryStream CreateTestStream(int length) private byte[] CreateTestBuffer(int length)
{ {
byte[] buffer = new byte[length]; byte[] buffer = new byte[length];
Random random = new(); this.bufferFiller.NextBytes(buffer);
random.NextBytes(buffer); return buffer;
return new MemoryStream(buffer);
} }
private MemoryStream CreateTestStream(int length)
=> new(this.CreateTestBuffer(length));
} }

Loading…
Cancel
Save