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

Improve float serialization, add .NET 5 Half support. #77

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
143 changes: 137 additions & 6 deletions NetSerializer/Primitives.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Diagnostics;
#if NETCOREAPP
using System.Text.Unicode;
using System.Buffers.Binary;
using System.Buffers;
#endif

Expand Down Expand Up @@ -219,24 +220,24 @@ public static void ReadPrimitive(Stream stream, out long value)
public static unsafe void WritePrimitive(Stream stream, float value)
{
uint v = *(uint*)(&value);
WriteVarint32(stream, v);
WriteUInt32(stream, v);
}

public static unsafe void ReadPrimitive(Stream stream, out float value)
{
uint v = ReadVarint32(stream);
uint v = ReadUInt32(stream);
value = *(float*)(&v);
}

public static unsafe void WritePrimitive(Stream stream, double value)
{
ulong v = *(ulong*)(&value);
WriteVarint64(stream, v);
WriteUInt64(stream, v);
}

public static unsafe void ReadPrimitive(Stream stream, out double value)
{
ulong v = ReadVarint64(stream);
ulong v = ReadUInt64(stream);
value = *(double*)(&v);
}
#else
Expand All @@ -255,16 +256,146 @@ public static void ReadPrimitive(Stream stream, out float value)
public static void WritePrimitive(Stream stream, double value)
{
ulong v = (ulong)BitConverter.DoubleToInt64Bits(value);
WriteVarint64(stream, v);
WriteUInt64(stream, v);
}

public static void ReadPrimitive(Stream stream, out double value)
{
ulong v = ReadVarint64(stream);
ulong v = ReadUInt64(stream);
value = BitConverter.Int64BitsToDouble((long)v);
}
#endif

private static void WriteUInt16(Stream stream, ushort value)
{
stream.WriteByte((byte) value);
stream.WriteByte((byte) (value >> 8));
}

private static ushort ReadUInt16(Stream stream)
{
ushort a = 0;

for (var i = 0; i < 16; i += 8)
{
var val = stream.ReadByte();
if (val == -1)
throw new EndOfStreamException();

a |= (ushort) (val << i);
}

return a;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't these two methods be part of the next commit ("Add support for .NET 5 Half type.")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was kind of debating that as well but figured this would make the diffs cleaner since now all these uint read/write methods are in the same commit.

Copy link
Owner

Choose a reason for hiding this comment

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

The reasons why I think it should not be part of the commit:

  • This commit is "improve float serialization", i.e. improve what we already have
  • The new methods are not used (so they cannot even improve the float serialization)
  • The commit message doesn't mention adding new feature
  • If you do add a note to the commit message, it would probably be something like this at the end of the message "Also, add xyz", which is a clear hint that the code is probably really not part of the commit.


// 32 and 64 bit variants use stackalloc when everything is available since it's faster.

#if !NETCOREAPP
private static void WriteUInt32(Stream stream, uint value)
{
stream.WriteByte((byte) value);
stream.WriteByte((byte) (value >> 8));
stream.WriteByte((byte) (value >> 16));
stream.WriteByte((byte) (value >> 24));
}

private static void WriteUInt64(Stream stream, ulong value)
{
stream.WriteByte((byte) value);
stream.WriteByte((byte) (value >> 8));
stream.WriteByte((byte) (value >> 16));
stream.WriteByte((byte) (value >> 24));
stream.WriteByte((byte) (value >> 32));
stream.WriteByte((byte) (value >> 40));
stream.WriteByte((byte) (value >> 48));
stream.WriteByte((byte) (value >> 56));
}

private static uint ReadUInt32(Stream stream)
{
uint a = 0;

for (var i = 0; i < 32; i += 8)
{
var val = stream.ReadByte();
if (val < 0)
throw new EndOfStreamException();

a |= (uint)val << i;
}

return a;
}

private static ulong ReadUInt64(Stream stream)
{
ulong a = 0;

for (var i = 0; i < 64; i += 8)
{
var val = stream.ReadByte();
if (val < 0)
throw new EndOfStreamException();

a |= (ulong)val << i;
}
PJB3005 marked this conversation as resolved.
Show resolved Hide resolved

return a;
}
#else
private static void WriteUInt32(Stream stream, uint value)
{
Span<byte> buf = stackalloc byte[4];
BinaryPrimitives.WriteUInt32LittleEndian(buf, value);

stream.Write(buf);
}

private static void WriteUInt64(Stream stream, ulong value)
{
Span<byte> buf = stackalloc byte[8];
BinaryPrimitives.WriteUInt64LittleEndian(buf, value);

stream.Write(buf);
}

private static uint ReadUInt32(Stream stream)
{
Span<byte> buf = stackalloc byte[4];
var wSpan = buf;

while (true)
{
var read = stream.Read(wSpan);
if (read == 0)
throw new EndOfStreamException();
if (read == wSpan.Length)
break;
wSpan = wSpan[read..];
}

return BinaryPrimitives.ReadUInt32LittleEndian(buf);
}

private static ulong ReadUInt64(Stream stream)
{
Span<byte> buf = stackalloc byte[8];
var wSpan = buf;

while (true)
{
var read = stream.Read(wSpan);
if (read == 0)
throw new EndOfStreamException();
if (read == wSpan.Length)
break;
wSpan = wSpan[read..];
}

return BinaryPrimitives.ReadUInt64LittleEndian(buf);
}
#endif

public static void WritePrimitive(Stream stream, DateTime value)
{
long v = value.ToBinary();
Expand Down