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.

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

S.IO.StringReader: Use ReadOnlySpan.IndexOfAny in ReadLine() for performance #60463

Merged
Merged
83 changes: 46 additions & 37 deletions src/libraries/System.Private.CoreLib/src/System/IO/StringReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ public class StringReader : TextReader
{
private string? _s;
private int _pos;
private int _length;

public StringReader(string s)
{
Expand All @@ -21,7 +20,6 @@ public StringReader(string s)
}

_s = s;
_length = s.Length;
}

public override void Close()
Expand All @@ -33,7 +31,6 @@ protected override void Dispose(bool disposing)
{
_s = null;
_pos = 0;
_length = 0;
base.Dispose(disposing);
}

Expand All @@ -44,16 +41,19 @@ protected override void Dispose(bool disposing)
//
public override int Peek()
{
if (_s == null)
string? s = _s;
if (s == null)
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
ThrowHelper.ThrowObjectDisposedException_ReaderClosed();
}
if (_pos == _length)

int pos = _pos;
if ((uint)pos < s.Length)
nietras marked this conversation as resolved.
Show resolved Hide resolved
{
return -1;
return s[pos];
}

return _s[_pos];
return -1;
}

// Reads the next character from the underlying string. The returned value
Expand All @@ -63,9 +63,9 @@ public override int Read()
{
if (_s == null)
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
ThrowHelper.ThrowObjectDisposedException_ReaderClosed();
}
if (_pos == _length)
if (_pos == _s.Length)
{
return -1;
}
Expand Down Expand Up @@ -98,10 +98,10 @@ public override int Read(char[] buffer, int index, int count)
}
if (_s == null)
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
ThrowHelper.ThrowObjectDisposedException_ReaderClosed();
}

int n = _length - _pos;
int n = _s.Length - _pos;
if (n > 0)
{
if (n > count)
Expand All @@ -126,10 +126,10 @@ public override int Read(Span<char> buffer)

if (_s == null)
nietras marked this conversation as resolved.
Show resolved Hide resolved
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
ThrowHelper.ThrowObjectDisposedException_ReaderClosed();
}

int n = _length - _pos;
int n = _s.Length - _pos;
if (n > 0)
{
if (n > buffer.Length)
Expand All @@ -150,7 +150,7 @@ public override string ReadToEnd()
{
if (_s == null)
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
ThrowHelper.ThrowObjectDisposedException_ReaderClosed();
}

string s;
Expand All @@ -160,10 +160,10 @@ public override string ReadToEnd()
}
else
{
s = _s.Substring(_pos, _length - _pos);
s = _s.Substring(_pos, _s.Length - _pos);
nietras marked this conversation as resolved.
Show resolved Hide resolved
}

_pos = _length;
_pos = _s.Length;
return s;
nietras marked this conversation as resolved.
Show resolved Hide resolved
}

Expand All @@ -175,38 +175,47 @@ public override string ReadToEnd()
//
public override string? ReadLine()
{
if (_s == null)
string? s = _s;
if (s == null)
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
ThrowHelper.ThrowObjectDisposedException_ReaderClosed();
}
var pos = _pos;
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
if ((uint)pos >= (uint)s.Length)
return null;
nietras marked this conversation as resolved.
Show resolved Hide resolved

int i = _pos;
while (i < _length)
ReadOnlySpan<char> remaining = s.AsSpan(pos);
int foundLineLength = remaining.IndexOfAny('\r', '\n');
if (foundLineLength >= 0)
{
char ch = _s[i];
if (ch == '\r' || ch == '\n')
string result = foundLineLength == 0
nietras marked this conversation as resolved.
Show resolved Hide resolved
? string.Empty
: new string(remaining.Slice(0, foundLineLength));

char ch = remaining[foundLineLength];
pos += foundLineLength + 1;
if (ch == '\r')
{
string result = _s.Substring(_pos, i - _pos);
_pos = i + 1;
if (ch == '\r' && _pos < _length && _s[_pos] == '\n')
if ((uint)pos < s.Length && s[pos] == '\n')
nietras marked this conversation as resolved.
Show resolved Hide resolved
{
_pos++;
++pos;
nietras marked this conversation as resolved.
Show resolved Hide resolved
}

return result;
}

i++;
_pos = pos;
return result;
}

if (i > _pos)
else if (remaining.Length == s.Length)
{
// Return source string if only one line avoiding allocation
_pos = s.Length;
return s;
}
else
{
string result = _s.Substring(_pos, i - _pos);
_pos = i;
string result = new string(remaining);
nietras marked this conversation as resolved.
Show resolved Hide resolved
_pos = s.Length;
return result;
}

return null;
}

#region Task based Async APIs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,12 @@ internal static void ThrowObjectDisposedException_FileClosed()
throw new ObjectDisposedException(null, SR.ObjectDisposed_FileClosed);
}

[DoesNotReturn]
internal static void ThrowObjectDisposedException_ReaderClosed()
nietras marked this conversation as resolved.
Show resolved Hide resolved
{
throw new ObjectDisposedException(null, SR.ObjectDisposed_ReaderClosed);
}

[DoesNotReturn]
internal static void ThrowObjectDisposedException(ExceptionResource resource)
{
Expand Down