SqlBytes.Stream should consider overriding Span Read/Write methods

[eerhardt]

SqlBytes.Stream can return a custom Stream class StreamOnSqlBytes and returns it to external callers. Since .NET Core 2.1, System.IO.Stream has had Read and Write overloads that accept a Span to read to / write from. However, the base Stream implementation isn't optimized. To get the best performance when a caller is using the Span-based APIs, derived implementations of Stream are expected to override the Span-based APIs and perform the operation on the Spans. See the System.IO.Stream section of #22387 for more information.

We should consider overriding the Span Read/Write methods on StreamOnSqlBytes:

runtime/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs

Lines 694 to 715 in 2a66135

	// The Read/Write/ReadByte/WriteByte simply delegates to SqlBytes
	public override int Read(byte[] buffer, int offset, int count)
	{
	CheckIfStreamClosed();
	ValidateBufferArguments(buffer, offset, count);
	int iBytesRead = (int)_sb.Read(_lPosition, buffer, offset, count);
	_lPosition += iBytesRead;
	return iBytesRead;
	}
	public override void Write(byte[] buffer, int offset, int count)
	{
	CheckIfStreamClosed();
	ValidateBufferArguments(buffer, offset, count);
	_sb.Write(_lPosition, buffer, offset, count);
	_lPosition += count;
	}

That way callers using the Span-based APIs get better performance - an ArrayPool buffer doesn't need to be rented, and the data copied twice.

Tagging subscribers to this area: @roji, @ajcvickers
See info in area-owners.md if you want to be subscribed.

Issue Details

---

SqlBytes.Stream can return a custom Stream class StreamOnSqlBytes and returns it to external callers. Since .NET Core 2.0, System.IO.Stream has had Read and Write overloads that accept a Span to read to / write from. However, the base Stream implementation isn't optimized. To get the best performance when a caller is using the Span-based APIs, derived implementations of Stream are expected to override the Span-based APIs and perform the operation on the Spans. See the System.IO.Stream section of #22387 for more information.

We should consider overriding the Span Read/Write methods on StreamOnSqlBytes:

runtime/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs

Lines 694 to 715 in 2a66135

	// The Read/Write/ReadByte/WriteByte simply delegates to SqlBytes
	public override int Read(byte[] buffer, int offset, int count)
	{
	CheckIfStreamClosed();
	ValidateBufferArguments(buffer, offset, count);
	int iBytesRead = (int)_sb.Read(_lPosition, buffer, offset, count);
	_lPosition += iBytesRead;
	return iBytesRead;
	}
	public override void Write(byte[] buffer, int offset, int count)
	{
	CheckIfStreamClosed();
	ValidateBufferArguments(buffer, offset, count);
	_sb.Write(_lPosition, buffer, offset, count);
	_lPosition += count;
	}

That way callers using the Span-based APIs get better performance - an ArrayPool buffer doesn't need to be rented, and the data copied twice.

Author:	eerhardt
Assignees:	-
Labels:	area-System.Data, tenet-performance
Milestone:	-

[hrrrrustic]

@eerhardt
This issue called should consider overriding, so some discussion required? Or it's fine for contributing implementation?

[eerhardt]

I think a contribution would be welcome. I don't think there is discussion other than "dev time".

But I'd let the area owners confirm:
@ajcvickers @DavoudEshtehari @David-Engel @roji

[David-Engel]

Looks all internal optimization to me, so I can't see any reason we wouldn't welcome a contribution here.

• @Wraith2 as our perf expert.