Derive from Stream in ReadOnlyMemoryStream and WritableMemoryStream

[jozkee]

Instead of deriving from MemoryStream.
Also support WritableMemoryStream.SetLength(long) since 1) is writable and seekable and 2) to properly align with MemoryStream user-buffer mode.

Fixes #130330.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR updates System.IO.ReadOnlyMemoryStream and System.IO.WritableMemoryStream to derive from Stream (instead of MemoryStream), removing MemoryStream-specific surface area and adding WritableMemoryStream.SetLength(long) support within the fixed-capacity backing buffer.

Changes:

• Change ReadOnlyMemoryStream / WritableMemoryStream base type from MemoryStream to Stream and update the reference assembly accordingly.
• Implement WritableMemoryStream.SetLength(long) to allow shrinking/growing length up to the fixed capacity (including zeroing newly exposed regions).
• Update stream conformance tests and add/adjust unit tests to validate fixed-capacity semantics (write/set-length beyond capacity, extending after shrink, etc.).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Re-implements WritableMemoryStream as a Stream, adds internal length tracking and SetLength behavior for fixed-capacity buffers.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Re-implements ReadOnlyMemoryStream as a Stream and ensures write operations throw.
src/libraries/System.Runtime/ref/System.Runtime.cs	Updates public API contract: types now derive from Stream and drop MemoryStream-specific members.
src/libraries/System.Private.CoreLib/src/Resources/Strings.resx	Adds a new resource string used for stream position range validation errors.
src/libraries/System.Runtime/tests/System.IO.Tests/WritableMemoryStream/WritableMemoryStreamTests.cs	Updates/extends unit coverage for fixed-capacity and new SetLength behaviors.
src/libraries/System.Runtime/tests/System.IO.Tests/WritableMemoryStream/WritableMemoryStreamConformanceTests.cs	Adjusts conformance harness for the new SetLength behavior (but currently skips empty-case growth coverage).
src/libraries/System.Runtime/tests/System.IO.Tests/ReadOnlyMemoryStream/ReadOnlyMemoryStreamTests.cs	Updates tests to validate write operations are not supported after base-type change.
src/libraries/System.Runtime/tests/System.IO.Tests/MemoryStream/MemoryStreamTests.cs	Adds coverage for user-buffer MemoryStream fixed-capacity behaviors to align expectations.
src/libraries/System.Memory/tests/ReadOnlyBuffer/NativeWritableMemoryStream.ConformanceTests.cs	Aligns native-buffer wrapper conformance expectations (but currently skips empty-case growth coverage).
src/libraries/Common/tests/StreamConformanceTests/System/IO/StreamConformanceTests.cs	Updates shared conformance tests: ensures write/read tests provision sufficient capacity and adds a grow-from-empty write test.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[adamsitnik]

LGTM, thank you for taking care of this @jozkee. I've left some comments, but please don't block the PR only because I don't like given micro optimization (if it turns out it's not needed anymore with runtime async, we are just going to send another PR and remove it from everywhere).

[adamsitnik]

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:

runtime/src/libraries/Common/src/System/Threading/Tasks/CachedCompletedInt32Task.cs

Lines 22 to 33 in 883277e

	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?

[jakobbotsch]

@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:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Lines 285 to 289 in e5fc966

	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.

[tannergooding]

Overall LGTM, but a few pieces of feedback/questions

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.