Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StreamOnSqlBytes Read/Write on Spans #86674

Merged
merged 11 commits into from
Jan 29, 2024
Merged

StreamOnSqlBytes Read/Write on Spans #86674

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6e6de4e
Implement span overloads
hrrrrustic May 23, 2023
17a8a9b
Add StreamOnBytes tests
hrrrrustic May 23, 2023
8043e95
small tests update
hrrrrustic May 23, 2023
fbe8e62
Reroute single byte read/write to span version
hrrrrustic May 23, 2023
cece5b2
Remove _rgbWorkBuf
hrrrrustic May 26, 2023
d6e3c28
Add WriteInternal
hrrrrustic May 26, 2023
19c9e51
Internal -> NoValidation
hrrrrustic May 26, 2023
b143fb8
Swap arguments
hrrrrustic May 26, 2023
12b6540
Swap methods
hrrrrustic May 26, 2023
970bff5
One more method swap
hrrrrustic May 26, 2023
ae5a71f
Merge branch 'main' into issue-69921
stephentoub Jan 29, 2024
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
165 changes: 112 additions & 53 deletions src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLBytes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,15 @@ public byte this[long offset]

_rgbWorkBuf ??= new byte[1];

Read(offset, _rgbWorkBuf, 0, 1);
Read(offset, _rgbWorkBuf.AsSpan(0, 1));
return _rgbWorkBuf[0];
}
set
{
_rgbWorkBuf ??= new byte[1];

_rgbWorkBuf[0] = value;
Write(offset, _rgbWorkBuf, 0, 1);
Write(offset, _rgbWorkBuf.AsSpan(0, 1));
}
}

Expand Down Expand Up @@ -290,6 +290,34 @@ public void SetLength(long value)
AssertValid();
}

private long ReadInternal(Span<byte> buffer, long offset)
{
if (_state == SqlBytesCharsState.Stream)
{
if (_stream!.Position != offset)
_stream.Seek(offset, SeekOrigin.Begin);
return _stream.Read(buffer);
}

// Adjust count based on data length
int count = Math.Min(buffer.Length, (int)(Length - offset));

Span<byte> span = _rgbBuf!.AsSpan((int)offset, count);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of the null-forgiving operator I'd prefer a Debug.Assert(_rgbBuf is not null), as in case it's actually null, then in CI the assert fails.
Same on other places.

Except it's really 100 % sure that it can't be null here, then ignore this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied all of ! from old array-based methods so I assume we are 100% sure

Copy link
Member

Choose a reason for hiding this comment

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

Please remove the ! and add the assert. The ! was ok when this was in the Read method that was also checking IsNull and throwing, but now this is reachable from an internal method that has no such check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But internal method also has IsNull check under your comment about arguments order 🤔

span.CopyTo(buffer);

return span.Length;
}

internal long Read(long offset, Span<byte> buffer)
{
if (IsNull)
throw new SqlNullValueException();

ArgumentOutOfRangeException.ThrowIfGreaterThan(offset, Length);
ArgumentOutOfRangeException.ThrowIfNegative(offset);

return ReadInternal(buffer, offset);
}
// Read data of specified length from specified offset into a buffer
public long Read(long offset, byte[] buffer, int offsetInBuffer, int count)
{
Expand All @@ -308,26 +336,69 @@ public long Read(long offset, byte[] buffer, int offsetInBuffer, int count)
ArgumentOutOfRangeException.ThrowIfNegative(count);
ArgumentOutOfRangeException.ThrowIfGreaterThan(count, buffer.Length - offsetInBuffer);

// Adjust count based on data length
if (count > Length - offset)
count = (int)(Length - offset);
return ReadInternal(buffer.AsSpan(offsetInBuffer, count), offset);
}

if (count != 0)
private void WriteInternal(long offset, ReadOnlySpan<byte> buffer)
{
if (IsNull)
{
switch (_state)
{
case SqlBytesCharsState.Stream:
if (_stream!.Position != offset)
_stream.Seek(offset, SeekOrigin.Begin);
count = _stream.Read(buffer, offsetInBuffer, count);
break;
// If NULL and there is buffer inside, we only allow writing from
// offset zero.
//
if (offset != 0)
throw new SqlTypeException(SR.SqlMisc_WriteNonZeroOffsetOnNullMessage);

default:
Array.Copy(_rgbBuf!, offset, buffer, offsetInBuffer, count);
break;
}
// treat as if our current length is zero.
// Note this has to be done after all inputs are validated, so that
// we won't throw exception after this point.
//
_lCurLen = 0;
_state = SqlBytesCharsState.Buffer;
}
else if (offset > _lCurLen)
{
// Don't allow writing from an offset that this larger than current length.
// It would leave uninitialized data in the buffer.
//
throw new SqlTypeException(SR.SqlMisc_WriteOffsetLargerThanLenMessage);
}

if (buffer.Length != 0)
{
Span<byte> span = _rgbBuf.AsSpan((int)offset, buffer.Length);
buffer.CopyTo(span);

// If the last position that has been written is after
// the current data length, reset the length
if (_lCurLen < offset + buffer.Length)
_lCurLen = offset + buffer.Length;
}
}

internal void Write(long offset, ReadOnlySpan<byte> buffer)
{
if (FStream())
{
if (_stream!.Position != offset)
_stream.Seek(offset, SeekOrigin.Begin);
_stream.Write(buffer);
}
else
{
if (_rgbBuf == null)
throw new SqlTypeException(SR.SqlMisc_NoBufferMessage);

ArgumentOutOfRangeException.ThrowIfNegative(offset);

if (offset > _rgbBuf.Length)
throw new SqlTypeException(SR.SqlMisc_BufferInsufficientMessage);

if (buffer.Length > _rgbBuf.Length - offset)
throw new SqlTypeException(SR.SqlMisc_BufferInsufficientMessage);

WriteInternal(offset, buffer);
}
return count;
}

// Write data of specified length into the SqlBytes from specified offset
Expand Down Expand Up @@ -360,38 +431,7 @@ public void Write(long offset, byte[] buffer, int offsetInBuffer, int count)
if (count > _rgbBuf.Length - offset)
throw new SqlTypeException(SR.SqlMisc_BufferInsufficientMessage);

if (IsNull)
{
// If NULL and there is buffer inside, we only allow writing from
// offset zero.
//
if (offset != 0)
throw new SqlTypeException(SR.SqlMisc_WriteNonZeroOffsetOnNullMessage);

// treat as if our current length is zero.
// Note this has to be done after all inputs are validated, so that
// we won't throw exception after this point.
//
_lCurLen = 0;
_state = SqlBytesCharsState.Buffer;
}
else if (offset > _lCurLen)
{
// Don't allow writing from an offset that this larger than current length.
// It would leave uninitialized data in the buffer.
//
throw new SqlTypeException(SR.SqlMisc_WriteOffsetLargerThanLenMessage);
}

if (count != 0)
{
Array.Copy(buffer, offsetInBuffer, _rgbBuf, offset, count);

// If the last position that has been written is after
// the current data length, reset the length
if (_lCurLen < offset + count)
_lCurLen = offset + count;
}
WriteInternal(offset, buffer.AsSpan(offsetInBuffer, count));
}

AssertValid();
Expand Down Expand Up @@ -686,17 +726,28 @@ public override long Seek(long offset, SeekOrigin origin)
return _lPosition;
}

private int ReadInternal(Span<byte> buffer)
{
int bytesRead = (int)_sb.Read(_lPosition, buffer);
_lPosition += bytesRead;

return bytesRead;
}
// 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 ReadInternal(buffer.AsSpan(offset, count));
}

public override int Read(Span<byte> buffer)
{
CheckIfStreamClosed();

return iBytesRead;
return ReadInternal(buffer);
}

public override void Write(byte[] buffer, int offset, int count)
Expand All @@ -709,6 +760,14 @@ public override void Write(byte[] buffer, int offset, int count)
_lPosition += count;
}

public override void Write(ReadOnlySpan<byte> buffer)
{
CheckIfStreamClosed();

_sb.Write(_lPosition, buffer);
_lPosition += buffer.Length;
}

public override int ReadByte()
{
CheckIfStreamClosed();
Expand Down
Loading