Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1094,16 +1094,16 @@ public virtual async Task Write_Read_Success(ReadWriteMode mode)
if (SkipOnWasi(mode)) return;

const int Length = 1024;
const int Copies = 3;

using Stream? stream = await CreateReadWriteStream();
using Stream? stream = await CreateReadWriteStream(new byte[Length * Copies]);
if (stream is null)
{
return;
}

byte[] expected = GetRandomBytes(Length);

const int Copies = 3;
for (int i = 0; i < Copies; i++)
{
await WriteAsync(mode, stream, expected, 0, expected.Length);
Expand All @@ -1120,6 +1120,42 @@ public virtual async Task Write_Read_Success(ReadWriteMode mode)
}
}

[Theory]
[MemberData(nameof(AllReadWriteModes))]
public virtual async Task Write_GrowsLength_Success(ReadWriteMode mode)
{
if (SkipOnWasi(mode)) return;

// Start from an empty stream. Streams that cannot produce an empty read-write instance
// (e.g. fixed-capacity streams that require initial data) return null and are skipped.
using Stream? stream = await CreateReadWriteStream();
if (stream is null)
{
return;
}

const int Length = 1024;
byte[] expected = GetRandomBytes(Length);

if (stream.CanSeek)
{
Assert.Equal(0, stream.Length);
}

await WriteAsync(mode, stream, expected, 0, expected.Length);

if (stream.CanSeek)
{
Assert.Equal(Length, stream.Position);
Assert.Equal(Length, stream.Length);

stream.Position = 0;
byte[] actual = new byte[Length];
Assert.Equal(Length, await ReadAllAsync(mode, stream, actual, 0, actual.Length));
AssertExtensions.SequenceEqual(expected, actual);
}
}

[Theory]
[MemberData(nameof(AllReadWriteModes))]
public virtual async Task Flush_ReadOnly_DoesntImpactReading(ReadWriteMode mode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace System.Memory.Tests
public class NativeWritableMemoryStreamConformanceTests : StandaloneStreamConformanceTests
{
protected override bool CanSeek => true;
protected override bool CanSetLength => false;
protected override bool NopFlushCompletesSynchronously => true;
protected override bool CanSetLengthGreaterThanCapacity => false;

Expand All @@ -20,17 +19,19 @@ public class NativeWritableMemoryStreamConformanceTests : StandaloneStreamConfor

protected override Task<Stream?> CreateReadWriteStreamCore(byte[]? initialData)
{
int dataLength = initialData?.Length ?? 0;
int length = dataLength == 0 ? 64 * 1024 : dataLength;
var manager = new System.Buffers.NativeMemoryManager(length);
// See WritableMemoryStreamConformanceTests: WritableMemoryStream is fixed-capacity, so the
// null case returns null and the grow-from-empty conformance tests are skipped.
if (initialData is null)
{
return Task.FromResult<Stream?>(null);
Comment thread
jozkee marked this conversation as resolved.
Comment thread
jozkee marked this conversation as resolved.
}

var manager = new System.Buffers.NativeMemoryManager(initialData.Length);
manager.GetSpan().Clear();

var inner = new WritableMemoryStream(manager.Memory);
if (dataLength != 0)
{
inner.Write(initialData!, 0, dataLength);
inner.Position = 0;
}
inner.Write(initialData, 0, initialData.Length);
inner.Position = 0;

return Task.FromResult<Stream?>(new NativeMemoryOwningStream(inner, manager));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,9 @@
<data name="ArgumentOutOfRange_StreamLength" xml:space="preserve">
<value>Stream length must be non-negative and less than the maximum array length {0} - origin.</value>
</data>
<data name="ArgumentOutOfRange_StreamPosition" xml:space="preserve">
<value>Stream position must be non-negative and less than or equal to {0}.</value>
</data>
<data name="ArgumentOutOfRange_UIntPtrMax" xml:space="preserve">
<value>The length of the buffer must be less than the maximum UIntPtr value for your platform.</value>
</data>
Expand Down
20 changes: 10 additions & 10 deletions src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,19 @@ namespace System.IO
// a stream "view" of the data.
public class MemoryStream : Stream
{
private byte[] _buffer; // Either allocated internally or externally.
private readonly int _origin; // For user-provided arrays, start at this origin
private protected int _position; // read/write head.
private protected int _length; // Number of bytes within the memory stream
private int _capacity; // length of usable portion of buffer for stream
private byte[] _buffer; // Either allocated internally or externally.
private readonly int _origin; // For user-provided arrays, start at this origin
private int _position; // read/write head.
private int _length; // Number of bytes within the memory stream
private int _capacity; // length of usable portion of buffer for stream
// Note that _capacity == _buffer.Length for non-user-provided byte[]'s

private bool _expandable; // User-provided buffers aren't expandable.
private protected bool _writable; // Can user write to this stream?
private readonly bool _exposable; // Whether the array can be returned to the user.
private protected bool _isOpen; // Is this stream open or closed?
private bool _expandable; // User-provided buffers aren't expandable.
private bool _writable; // Can user write to this stream?
private readonly bool _exposable; // Whether the array can be returned to the user.
private bool _isOpen; // Is this stream open or closed?

private protected CachedCompletedInt32Task _lastReadTask; // The last successful task returned from ReadAsync
private CachedCompletedInt32Task _lastReadTask; // The last successful task returned from ReadAsync

public MemoryStream()
: this(0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,78 @@
namespace System.IO
{
/// <summary>
/// Provides a seekable, read-only <see cref="MemoryStream"/> over a <see cref="ReadOnlyMemory{Byte}"/>.
/// Provides a seekable, read-only <see cref="Stream"/> over a <see cref="ReadOnlyMemory{Byte}"/>.
/// </summary>
/// <remarks>
/// <para>The stream cannot be written to. <see cref="MemoryStream.CanWrite"/> always returns <see langword="false"/>.</para>
/// <para><see cref="MemoryStream.GetBuffer"/> throws and <see cref="MemoryStream.TryGetBuffer"/> returns <see langword="false"/>.</para>
/// <para>The stream cannot be written to. <see cref="CanWrite"/> always returns <see langword="false"/>.</para>
/// </remarks>
public sealed class ReadOnlyMemoryStream : MemoryStream
Comment thread
tannergooding marked this conversation as resolved.
public sealed class ReadOnlyMemoryStream : Stream
{
private ReadOnlyMemory<byte> _memory;
private int _position;
private bool _isOpen;
private CachedCompletedInt32Task _lastReadTask; // The last successful task returned from ReadAsync

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subjective: I am not a big fan of this performance optimization.

@jakobbotsch what some of the BCL Streams do is that they keep a cached Task<int> instance and re-use it if the following read returned as many bytes as the previous one:

public Task<int> GetTask(int result)
{
if (_task is Task<int> task)
{
Debug.Assert(task.IsCompletedSuccessfully, "Expected that a stored last task completed successfully");
if (task.Result == result)
{
return task;
}
}
return _task = Task.FromResult(result);

So for example, when we open a large file and keep reading the contents using the buffer of the same size, we avoid allocating for all reads except of the first one (it initializes the cache) and the last one (assuming the number of bytes in 0 or just different than buffer size)

@jakobbotsch do we still need such a micro optimization with runtime async enabled?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakobbotsch do we still need such a micro optimization with runtime async enabled?

No, this pattern is actually quite detrimental to performance. Runtime async recognizes return Task.FromResult(x) specially, and it will turn it into something that never allocates if things are runtime async.

Similarly bad is stuff like this:

ValueTask<int> readResult = ReadAsync(new Memory<byte>(buffer, offset, count), cancellationToken);
return readResult.IsCompletedSuccessfully
? _lastSyncCompletedReadTask.GetTask(readResult.Result)
: readResult.AsTask();

This forces ValueTask<int> to be created instead of remaining as runtime async throughout if this just returned ReadAsync(...).AsTask() directly.


/// <summary>
/// Initializes a new instance of the <see cref="ReadOnlyMemoryStream"/> class over the specified <see cref="ReadOnlyMemory{Byte}"/>.
/// </summary>
/// <param name="source">The <see cref="ReadOnlyMemory{Byte}"/> to wrap.</param>
public ReadOnlyMemoryStream(ReadOnlyMemory<byte> source) : base()
public ReadOnlyMemoryStream(ReadOnlyMemory<byte> source)
{
_writable = false;
_memory = source;
_length = source.Length;
_isOpen = true;
}

/// <inheritdoc/>
public override int Capacity
public override bool CanRead => _isOpen;

/// <inheritdoc/>
public override bool CanSeek => _isOpen;

/// <inheritdoc/>
public override bool CanWrite => false;

/// <inheritdoc/>
public override long Length
{
get
{
EnsureNotClosed();
return _memory.Length;
}
set => throw new NotSupportedException(SR.NotSupported_MemStreamNotExpandable);
}

/// <inheritdoc/>
public override byte[] GetBuffer() =>
throw new UnauthorizedAccessException(SR.UnauthorizedAccess_MemStreamBuffer);
public override long Position
{
get
{
EnsureNotClosed();
return _position;
}
set
{
ArgumentOutOfRangeException.ThrowIfNegative(value);
EnsureNotClosed();

if (value > int.MaxValue)
Comment thread
tannergooding marked this conversation as resolved.
{
throw new ArgumentOutOfRangeException(nameof(value), SR.Format(SR.ArgumentOutOfRange_StreamPosition, int.MaxValue));
Comment thread
tannergooding marked this conversation as resolved.
}

_position = (int)value;
}
}

/// <inheritdoc/>
public override bool TryGetBuffer(out ArraySegment<byte> buffer)
public override void Flush()
{
buffer = default;
return false;
}

/// <inheritdoc/>
public override Task FlushAsync(CancellationToken cancellationToken) =>
cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.CompletedTask;

/// <inheritdoc/>
public override int ReadByte()
{
Expand Down Expand Up @@ -121,6 +150,34 @@ public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken
return new ValueTask<int>(Read(buffer.Span));
}

/// <inheritdoc/>
public override long Seek(long offset, SeekOrigin origin)
{
EnsureNotClosed();

int loc = origin switch
{
SeekOrigin.Begin => 0,
SeekOrigin.Current => _position,
SeekOrigin.End => _memory.Length,
_ => throw new ArgumentException(SR.Argument_InvalidSeekOrigin)
};

if (offset > int.MaxValue - loc)
{
throw new ArgumentOutOfRangeException(nameof(offset), SR.Format(SR.ArgumentOutOfRange_StreamPosition, int.MaxValue));
}

int tempPosition = unchecked(loc + (int)offset);
if (unchecked(loc + offset) < 0 || tempPosition < 0)
{
throw new IOException(SR.IO_SeekBeforeBegin);
}

_position = tempPosition;
return _position;
}

/// <inheritdoc/>
public override void CopyTo(Stream destination, int bufferSize)
{
Expand Down Expand Up @@ -157,32 +214,29 @@ public override Task CopyToAsync(Stream destination, int bufferSize, Cancellatio
}

/// <inheritdoc/>
public override byte[] ToArray()
{
EnsureNotClosed();
if (_memory.Length == 0)
{
return Array.Empty<byte>();
}
public override void SetLength(long value) => throw new NotSupportedException(SR.NotSupported_UnwritableStream);

byte[] copy = GC.AllocateUninitializedArray<byte>(_memory.Length);
_memory.Span.CopyTo(copy);
return copy;
}
/// <inheritdoc/>
public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(SR.NotSupported_UnwritableStream);
Comment thread
jozkee marked this conversation as resolved.
Comment thread
jozkee marked this conversation as resolved.
Comment thread
jozkee marked this conversation as resolved.

/// <inheritdoc/>
public override void WriteTo(Stream stream)
{
ArgumentNullException.ThrowIfNull(stream);
EnsureNotClosed();
public override void Write(ReadOnlySpan<byte> buffer) => throw new NotSupportedException(SR.NotSupported_UnwritableStream);

stream.Write(_memory.Span);
}
/// <inheritdoc/>
public override void WriteByte(byte value) => throw new NotSupportedException(SR.NotSupported_UnwritableStream);

/// <inheritdoc/>
public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => throw new NotSupportedException(SR.NotSupported_UnwritableStream);

/// <inheritdoc/>
public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default) => throw new NotSupportedException(SR.NotSupported_UnwritableStream);

/// <inheritdoc/>
protected override void Dispose(bool disposing)
{
_isOpen = false;
_memory = default;
_lastReadTask = default;
base.Dispose(disposing);
}

Expand Down
Loading
Loading