Skip to content

Commit

Permalink
Improve BinaryReader.ReadUInt32() perf by 30% when using MemoryStream (…
Browse files Browse the repository at this point in the history 
…dotnet#22102)

BinaryReader.ReadInt32() has nice optimization which was missing from the ReadUInt32() version. Now both implementations are aligned.

ReadUInt32 and all the 64 bits types are now 30% to 50% faster than original implementation.
  • Loading branch information
@TomerWeisberg @jkotas
TomerWeisberg authored and jkotas committed Jan 28, 2019
1 parent 731b40d commit 87cc716
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 103 deletions.
13 changes: 7 additions & 6 deletions src/System.Private.CoreLib/shared/System/Decimal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Buffers.Binary;
using System.Diagnostics;
using System.Globalization;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -553,13 +554,13 @@ internal static void GetBytes(in decimal d, byte[] buffer)
buffer[15] = (byte)(d.flags >> 24);
}

internal static decimal ToDecimal(byte[] buffer)
internal static decimal ToDecimal(ReadOnlySpan<byte> span)
{
Debug.Assert((buffer != null && buffer.Length >= 16), "[ToDecimal]buffer != null && buffer.Length >= 16");
int lo = ((int)buffer[0]) | ((int)buffer[1] << 8) | ((int)buffer[2] << 16) | ((int)buffer[3] << 24);
int mid = ((int)buffer[4]) | ((int)buffer[5] << 8) | ((int)buffer[6] << 16) | ((int)buffer[7] << 24);
int hi = ((int)buffer[8]) | ((int)buffer[9] << 8) | ((int)buffer[10] << 16) | ((int)buffer[11] << 24);
int flags = ((int)buffer[12]) | ((int)buffer[13] << 8) | ((int)buffer[14] << 16) | ((int)buffer[15] << 24);
Debug.Assert((span.Length >= 16), "span.Length >= 16");
int lo = BinaryPrimitives.ReadInt32LittleEndian(span);
int mid = BinaryPrimitives.ReadInt32LittleEndian(span.Slice(4));
int hi = BinaryPrimitives.ReadInt32LittleEndian(span.Slice(8));
int flags = BinaryPrimitives.ReadInt32LittleEndian(span.Slice(12));
return new decimal(lo, mid, hi, flags);
}

Expand Down
153 changes: 61 additions & 92 deletions src/System.Private.CoreLib/shared/System/IO/BinaryReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
**
============================================================*/

using System.Buffers.Binary;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;

Expand Down Expand Up @@ -102,6 +104,7 @@ protected virtual void Dispose(bool disposing)
copyOfStream.Close();
}
}
_isMemoryStream = false;
_stream = null;
_buffer = null;
_decoder = null;
Expand Down Expand Up @@ -220,15 +223,12 @@ public virtual int Read()
return _singleChar[0];
}

public virtual bool ReadBoolean()
{
FillBuffer(1);
return (_buffer[0] != 0);
}
public virtual byte ReadByte() => InternalReadByte();

public virtual byte ReadByte()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private byte InternalReadByte()
{
// Inlined to avoid some method call overhead with FillBuffer.
// Inlined to avoid some method call overhead with InternalRead.
if (_stream == null)
{
throw Error.GetFileNotOpen();
Expand All @@ -244,11 +244,8 @@ public virtual byte ReadByte()
}

[CLSCompliant(false)]
public virtual sbyte ReadSByte()
{
FillBuffer(1);
return (sbyte)(_buffer[0]);
}
public virtual sbyte ReadSByte() => (sbyte)InternalReadByte();
public virtual bool ReadBoolean() => InternalReadByte() != 0;

public virtual char ReadChar()
{
Expand All @@ -260,94 +257,26 @@ public virtual char ReadChar()
return (char)value;
}

public virtual short ReadInt16()
{
FillBuffer(2);
return (short)(_buffer[0] | _buffer[1] << 8);
}
public virtual short ReadInt16() => BinaryPrimitives.ReadInt16LittleEndian(InternalRead(2));

[CLSCompliant(false)]
public virtual ushort ReadUInt16()
{
FillBuffer(2);
return (ushort)(_buffer[0] | _buffer[1] << 8);
}

public virtual int ReadInt32()
{
if (_isMemoryStream)
{
if (_stream == null)
{
throw Error.GetFileNotOpen();
}

// read directly from MemoryStream buffer
MemoryStream mStream = _stream as MemoryStream;
Debug.Assert(mStream != null, "_stream as MemoryStream != null");

return mStream.InternalReadInt32();
}
else
{
FillBuffer(4);
return (int)(_buffer[0] | _buffer[1] << 8 | _buffer[2] << 16 | _buffer[3] << 24);
}
}
public virtual ushort ReadUInt16() => BinaryPrimitives.ReadUInt16LittleEndian(InternalRead(2));

public virtual int ReadInt32() => BinaryPrimitives.ReadInt32LittleEndian(InternalRead(4));
[CLSCompliant(false)]
public virtual uint ReadUInt32()
{
FillBuffer(4);
return (uint)(_buffer[0] | _buffer[1] << 8 | _buffer[2] << 16 | _buffer[3] << 24);
}

public virtual long ReadInt64()
{
FillBuffer(8);
uint lo = (uint)(_buffer[0] | _buffer[1] << 8 |
_buffer[2] << 16 | _buffer[3] << 24);
uint hi = (uint)(_buffer[4] | _buffer[5] << 8 |
_buffer[6] << 16 | _buffer[7] << 24);
return (long)((ulong)hi) << 32 | lo;
}

public virtual uint ReadUInt32() => BinaryPrimitives.ReadUInt32LittleEndian(InternalRead(4));
public virtual long ReadInt64() => BinaryPrimitives.ReadInt64LittleEndian(InternalRead(8));
[CLSCompliant(false)]
public virtual ulong ReadUInt64()
{
FillBuffer(8);
uint lo = (uint)(_buffer[0] | _buffer[1] << 8 |
_buffer[2] << 16 | _buffer[3] << 24);
uint hi = (uint)(_buffer[4] | _buffer[5] << 8 |
_buffer[6] << 16 | _buffer[7] << 24);
return ((ulong)hi) << 32 | lo;
}

public virtual unsafe float ReadSingle()
{
FillBuffer(4);
uint tmpBuffer = (uint)(_buffer[0] | _buffer[1] << 8 | _buffer[2] << 16 | _buffer[3] << 24);
return *((float*)&tmpBuffer);
}

public virtual unsafe double ReadDouble()
{
FillBuffer(8);
uint lo = (uint)(_buffer[0] | _buffer[1] << 8 |
_buffer[2] << 16 | _buffer[3] << 24);
uint hi = (uint)(_buffer[4] | _buffer[5] << 8 |
_buffer[6] << 16 | _buffer[7] << 24);

ulong tmpBuffer = ((ulong)hi) << 32 | lo;
return *((double*)&tmpBuffer);
}
public virtual ulong ReadUInt64() => BinaryPrimitives.ReadUInt64LittleEndian(InternalRead(8));
public virtual unsafe float ReadSingle() => BitConverter.Int32BitsToSingle(BinaryPrimitives.ReadInt32LittleEndian(InternalRead(4)));
public virtual unsafe double ReadDouble() => BitConverter.Int64BitsToDouble(BinaryPrimitives.ReadInt64LittleEndian(InternalRead(8)));

public virtual decimal ReadDecimal()
{
FillBuffer(16);
ReadOnlySpan<byte> span = InternalRead(16);
try
{
return decimal.ToDecimal(_buffer);
return decimal.ToDecimal(span);
}
catch (ArgumentException e)
{
Expand Down Expand Up @@ -492,8 +421,8 @@ private int InternalReadChars(Span<char> buffer)
byte[] byteBuffer = null;
if (_isMemoryStream)
{
MemoryStream mStream = _stream as MemoryStream;
Debug.Assert(mStream != null, "_stream as MemoryStream != null");
Debug.Assert(_stream is MemoryStream);
MemoryStream mStream = (MemoryStream)_stream;

position = mStream.InternalGetPosition();
numBytes = mStream.InternalEmulateRead(numBytes);
Expand Down Expand Up @@ -649,6 +578,46 @@ public virtual byte[] ReadBytes(int count)
return result;
}

private ReadOnlySpan<byte> InternalRead(int numBytes)
{
Debug.Assert(numBytes >= 2 && numBytes <= 16, "value of 1 should use ReadByte. value > 16 requires to change the minimal _buffer size");

if (_isMemoryStream)
{
// no need to check if _stream == null as we will never have null _stream when _isMemoryStream = true

// read directly from MemoryStream buffer
Debug.Assert(_stream is MemoryStream);
return ((MemoryStream)_stream).InternalReadSpan(numBytes);
}
else
{
if (_stream == null)
{
throw Error.GetFileNotOpen();
}

int bytesRead = 0;
int n = 0;

do
{
n = _stream.Read(_buffer, bytesRead, numBytes - bytesRead);
if (n == 0)
{
throw Error.GetEndOfFile();
}
bytesRead += n;
} while (bytesRead < numBytes);

return _buffer;
}
}

// FillBuffer is not performing well when reading from MemoryStreams as it is using the public Stream interface.
// We introduced new function InternalRead which can work directly on the MemoryStream internal buffer or using the public Stream
// interface when working with all other streams. This function is not needed anymore but we decided not to delete it for compatibility
// reasons. More about the subject in: https://github.com/dotnet/coreclr/pull/22102
protected virtual void FillBuffer(int numBytes)
{
if (_buffer != null && (numBytes < 0 || numBytes > _buffer.Length))
Expand Down
16 changes: 11 additions & 5 deletions src/System.Private.CoreLib/shared/System/IO/MemoryStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,18 +226,24 @@ internal int InternalGetPosition()
return _position;
}

// PERF: Takes out Int32 as fast as possible
internal int InternalReadInt32()
// PERF: Expose internal buffer for BinaryReader instead of going via the regular Stream interface which requires to copy the data out
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal ReadOnlySpan<byte> InternalReadSpan(int count)
{
EnsureNotClosed();

int pos = (_position += 4); // use temp to avoid a race condition
if (pos > _length)
int origPos = _position;
int newPos = origPos + count;

if ((uint)newPos > (uint)_length)
{
_position = _length;
throw Error.GetEndOfFile();
}
return (int)(_buffer[pos - 4] | _buffer[pos - 3] << 8 | _buffer[pos - 2] << 16 | _buffer[pos - 1] << 24);

var span = new ReadOnlySpan<byte>(_buffer, origPos, count);
_position = newPos;
return span;
}

// PERF: Get actual length of bytes available for read; do sanity checks; shift position - i.e. everything except actual copying bytes
Expand Down

0 comments on commit 87cc716

Please sign in to comment.