Skip to content

Use Span<T>-based methods in System.Net.Mail #115111

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

Merged
merged 12 commits into from
May 5, 2025
Merged

Use Span<T>-based methods in System.Net.Mail #115111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e742ffd
Span-based IByteEncoder
rzikm Apr 28, 2025
c9cabdd
IEncodableStream.DecodeBytes
rzikm Apr 28, 2025
a020b34
IEncodableStream.EncodeBytes
rzikm Apr 28, 2025
284b521
DelegatedStream and derived implementations.
rzikm Apr 28, 2025
f96c60e
Minor changes
rzikm Apr 28, 2025
455ae2e
Update src/libraries/System.Net.Mail/src/System/Net/Base64Stream.cs
rzikm Apr 28, 2025
86b8f0b
Apply suggestions from code review
rzikm Apr 28, 2025
02672d5
Code review feedback
rzikm Apr 29, 2025
457136f
Removed TODO
rzikm Apr 30, 2025
4ce61e5
Minor changes
rzikm Apr 30, 2025
2832126
Merge branch 'main' into smtp-spanify
rzikm Apr 30, 2025
21f436a
Consistent exceptions
rzikm May 5, 2025
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
136 changes: 55 additions & 81 deletions src/libraries/System.Net.Mail/src/System/Net/Base64Stream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Buffers;

namespace System.Net
{
Expand All @@ -20,7 +21,7 @@ internal sealed class Base64Stream : DelegatedStream, IEncodableStream
255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 62, 255, 255, 255, 63, // 2
52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 255, 255, 255, 255, 255, 255, // 3
255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 4
15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 255, 255, 255, 255, 255, // 5
15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 255, 255, 255, 255, 255, // 5
255, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, // 6
41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 255, 255, 255, 255, 255, // 7
255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, // 8
Expand Down Expand Up @@ -52,6 +53,9 @@ internal Base64Stream(Base64WriteStateInfo writeStateInfo) : base(new MemoryStre
_encoder = new Base64Encoder(_writeState, writeStateInfo.MaxLineLength);
}

public override bool CanRead => BaseStream.CanRead;
public override bool CanWrite => BaseStream.CanWrite;

private ReadStateInfo ReadState => _readState ??= new ReadStateInfo();

internal WriteStateInfoBase WriteState
Expand All @@ -63,12 +67,6 @@ internal WriteStateInfoBase WriteState
}
}

public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) =>
TaskToAsyncResult.Begin(ReadAsync(buffer, offset, count, CancellationToken.None), callback, state);

public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) =>
TaskToAsyncResult.Begin(WriteAsync(buffer, offset, count, CancellationToken.None), callback, state);

public override void Close()
{
if (_writeState != null && WriteState.Length > 0)
Expand All @@ -80,14 +78,14 @@ public override void Close()
base.Close();
}

public unsafe int DecodeBytes(byte[] buffer, int offset, int count)
public unsafe int DecodeBytes(Span<byte> buffer)
Copy link
Member

Choose a reason for hiding this comment

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

Something to consider for the future if we're cleaning this up is to try and replace all of this with built-in base64 helpers.

{
fixed (byte* pBuffer = buffer)
{
byte* start = pBuffer + offset;
byte* start = pBuffer;
byte* source = start;
byte* dest = start;
byte* end = start + count;
byte* end = start + buffer.Length;

while (source < end)
{
Expand Down Expand Up @@ -133,24 +131,18 @@ public unsafe int DecodeBytes(byte[] buffer, int offset, int count)
}
}

public int EncodeBytes(byte[] buffer, int offset, int count) =>
EncodeBytes(buffer, offset, count, true, true);
public int EncodeBytes(ReadOnlySpan<byte> buffer) =>
_encoder.EncodeBytes(buffer, true, true);

internal int EncodeBytes(byte[] buffer, int offset, int count, bool dontDeferFinalBytes, bool shouldAppendSpaceToCRLF)
internal int EncodeBytes(ReadOnlySpan<byte> buffer, bool dontDeferFinalBytes, bool shouldAppendSpaceToCRLF)
{
return _encoder.EncodeBytes(buffer, offset, count, dontDeferFinalBytes, shouldAppendSpaceToCRLF);
return _encoder.EncodeBytes(buffer, dontDeferFinalBytes, shouldAppendSpaceToCRLF);
}

public int EncodeString(string value, Encoding encoding) => _encoder.EncodeString(value, encoding);

public string GetEncodedString() => _encoder.GetEncodedString();

public override int EndRead(IAsyncResult asyncResult) =>
TaskToAsyncResult.End<int>(asyncResult);

public override void EndWrite(IAsyncResult asyncResult) =>
TaskToAsyncResult.End(asyncResult);

public override void Flush()
{
if (_writeState != null && WriteState.Length > 0)
Expand All @@ -163,90 +155,78 @@ public override void Flush()

public override async Task FlushAsync(CancellationToken cancellationToken)
{
if (_writeState != null && WriteState.Length > 0)
{
await base.WriteAsync(WriteState.Buffer.AsMemory(0, WriteState.Length), cancellationToken).ConfigureAwait(false);
WriteState.Reset();
}

await FlushInternalAsync(cancellationToken).ConfigureAwait(false);
await base.FlushAsync(cancellationToken).ConfigureAwait(false);
}

private void FlushInternal()
{
base.Write(WriteState.Buffer, 0, WriteState.Length);
BaseStream.Write(WriteState.Buffer.AsSpan(0, WriteState.Length));
WriteState.Reset();
}

public override int Read(byte[] buffer, int offset, int count)
private async ValueTask FlushInternalAsync(CancellationToken cancellationToken)
{
ValidateBufferArguments(buffer, offset, count);
await BaseStream.WriteAsync(WriteState.Buffer.AsMemory(0, WriteState.Length), cancellationToken).ConfigureAwait(false);
WriteState.Reset();
}

protected override int ReadInternal(Span<byte> buffer)
{
while (true)
{
// read data from the underlying stream
int read = base.Read(buffer, offset, count);
int read = BaseStream.Read(buffer);

// if the underlying stream returns 0 then there
// is no more data - ust return 0.
// is no more data - just return 0.
if (read == 0)
{
return 0;
}

// while decoding, we may end up not having
// any bytes to return pending additional data
// from the underlying stream.
read = DecodeBytes(buffer, offset, read);
// Decode the read bytes and update the input buffer with decoded bytes
read = DecodeBytes(buffer.Slice(0, read));
if (read > 0)
{
return read;
}
}
}
public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
ValidateBufferArguments(buffer, offset, count);
return ReadAsyncCore(buffer, offset, count, cancellationToken);

async Task<int> ReadAsyncCore(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
protected override async ValueTask<int> ReadAsyncInternal(Memory<byte> buffer, CancellationToken cancellationToken = default)
{
while (true)
{
while (true)
{
// read data from the underlying stream
int read = await base.ReadAsync(buffer.AsMemory(offset, count), cancellationToken).ConfigureAwait(false);
// read data from the underlying stream
int read = await BaseStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);

// if the underlying stream returns 0 then there
// is no more data - ust return 0.
if (read == 0)
{
return 0;
}
// if the underlying stream returns 0 then there
// is no more data - just return 0.
if (read == 0)
{
return 0;
}

// while decoding, we may end up not having
// any bytes to return pending additional data
// from the underlying stream.
read = DecodeBytes(buffer, offset, read);
if (read > 0)
{
return read;
}
// Decode the read bytes and update the input buffer with decoded bytes
read = DecodeBytes(buffer.Span.Slice(0, read));
if (read > 0)
{
return read;
}
}
}

public override void Write(byte[] buffer, int offset, int count)
protected override void WriteInternal(ReadOnlySpan<byte> buffer)
{
ValidateBufferArguments(buffer, offset, count);

int written = 0;

// do not append a space when writing from a stream since this means
// it's writing the email body
while (true)
{
written += EncodeBytes(buffer, offset + written, count - written, false, false);
if (written < count)
written += EncodeBytes(buffer.Slice(written), false, false);
if (written < buffer.Length)
{
FlushInternal();
}
Expand All @@ -257,28 +237,22 @@ public override void Write(byte[] buffer, int offset, int count)
}
}

public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
protected override async ValueTask WriteAsyncInternal(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
{
ValidateBufferArguments(buffer, offset, count);
return WriteAsyncCore(buffer, offset, count, cancellationToken);
int written = 0;

async Task WriteAsyncCore(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
// do not append a space when writing from a stream since this means
// it's writing the email body
while (true)
{
int written = 0;

// do not append a space when writing from a stream since this means
// it's writing the email body
while (true)
written += EncodeBytes(buffer.Span.Slice(written), false, false);
if (written < buffer.Length)
{
written += EncodeBytes(buffer, offset + written, count - written, false, false);
if (written < count)
{
await FlushAsync(cancellationToken).ConfigureAwait(false);
}
else
{
break;
}
await FlushInternalAsync(cancellationToken).ConfigureAwait(false);
}
else
{
break;
}
}
}
Expand Down
87 changes: 34 additions & 53 deletions src/libraries/System.Net.Mail/src/System/Net/BufferedReadStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using System.Buffers;

namespace System.Net
{
Expand All @@ -23,96 +24,76 @@ internal BufferedReadStream(Stream stream, bool readMore) : base(stream)
_readMore = readMore;
}

public override bool CanWrite
{
get
{
return false;
}
}
public override bool CanWrite => false;
public override bool CanRead => BaseStream.CanRead;

public override bool CanSeek
{
get
{
return false;
}
}

public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) =>
TaskToAsyncResult.Begin(ReadAsync(buffer, offset, count, CancellationToken.None), callback, state);
public override bool CanSeek => false;

public override int EndRead(IAsyncResult asyncResult) =>
TaskToAsyncResult.End<int>(asyncResult);

public override int Read(byte[] buffer, int offset, int count)
protected override int ReadInternal(Span<byte> buffer)
{
int read = 0;
if (_storedOffset < _storedLength)
{
read = Math.Min(count, _storedLength - _storedOffset);
Buffer.BlockCopy(_storedBuffer!, _storedOffset, buffer, offset, read);
int read = Math.Min(buffer.Length, _storedLength - _storedOffset);
_storedBuffer.AsSpan(_storedOffset, read).CopyTo(buffer);
_storedOffset += read;
if (read == count || !_readMore)
if (read == buffer.Length || !_readMore)
{
return read;
}

offset += read;
count -= read;
// Need to read more from the underlying stream
return read + BaseStream.Read(buffer.Slice(read));
}
return read + base.Read(buffer, offset, count);

return BaseStream.Read(buffer);
}

public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
protected override ValueTask<int> ReadAsyncInternal(Memory<byte> buffer, CancellationToken cancellationToken = default)
{
int read;
if (_storedOffset >= _storedLength)
{
return base.ReadAsync(buffer, offset, count, cancellationToken);
return BaseStream.ReadAsync(buffer, cancellationToken);
}

read = Math.Min(count, _storedLength - _storedOffset);
Buffer.BlockCopy(_storedBuffer!, _storedOffset, buffer, offset, read);
int read = Math.Min(buffer.Length, _storedLength - _storedOffset);
_storedBuffer.AsMemory(_storedOffset, read).CopyTo(buffer);
_storedOffset += read;
if (read == count || !_readMore)
if (read == buffer.Length || !_readMore)
{
return Task.FromResult<int>(read);
return new ValueTask<int>(read);
}

offset += read;
count -= read;

return ReadMoreAsync(read, buffer, offset, count, cancellationToken);
// Need to read more from the underlying stream
return ReadMoreAsync(read, buffer.Slice(read), cancellationToken);
}

private async Task<int> ReadMoreAsync(int bytesAlreadyRead, byte[] buffer, int offset, int count, CancellationToken cancellationToken)
private async ValueTask<int> ReadMoreAsync(int bytesAlreadyRead, Memory<byte> buffer, CancellationToken cancellationToken)
{
int returnValue = await base.ReadAsync(buffer.AsMemory(offset, count), cancellationToken).ConfigureAwait(false);
int returnValue = await BaseStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);
return bytesAlreadyRead + returnValue;
}

public override int ReadByte()
protected override void WriteInternal(ReadOnlySpan<byte> buffer)
{
if (_storedOffset < _storedLength)
{
return _storedBuffer![_storedOffset++];
}
else
{
return base.ReadByte();
}
throw new NotImplementedException();
}

protected override ValueTask WriteAsyncInternal(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
{
throw new NotImplementedException();
}

// adds additional content to the beginning of the buffer
// so the layout of the storedBuffer will be
// <buffer><existingBuffer>
// after calling push
internal void Push(byte[] buffer, int offset, int count)
internal void Push(ReadOnlySpan<byte> buffer)
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion for future: This looks replaceable with our ArrayBuffer helper

{
if (count == 0)
if (buffer.Length == 0)
return;

int count = buffer.Length;

if (_storedOffset == _storedLength)
{
if (_storedBuffer == null || _storedBuffer.Length < count)
Expand Down Expand Up @@ -146,7 +127,7 @@ internal void Push(byte[] buffer, int offset, int count)
}
}

Buffer.BlockCopy(buffer, offset, _storedBuffer!, _storedOffset, count);
buffer.CopyTo(_storedBuffer.AsSpan(_storedOffset));
}
}
}
Loading
Loading