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
Show file tree
Hide file tree
Changes from 4 commits
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
26 changes: 26 additions & 0 deletions NetSerializer.UnitTests/NetSerializer.UnitTests.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net45;netcoreapp3.1;net5.0</TargetFrameworks>

<IsPackable>false</IsPackable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="NUnit" Version="3.12.0" />
<PackageReference Include="NUnit3TestAdapter" Version="3.16.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
<PackageReference Include="NUnit.Analyzers" Version="0.5.0" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\NetSerializer\NetSerializer.csproj" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
</ItemGroup>
</Project>
103 changes: 103 additions & 0 deletions NetSerializer.UnitTests/PrimitivesTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
using System;
using System.IO;
using NUnit.Framework;

namespace NetSerializer.UnitTests
{
[TestFixture]
[TestOf(typeof(Primitives))]
[Parallelizable(ParallelScope.All)]
public class PrimitivesTest
{
#if NO_UNSAFE
[Ignore("Float tests are inacurrate due to rounding when NO_UNSAFE is enabled.")]
#endif
[Test]
[TestCase(0)]
[TestCase(1)]
[TestCase(123.4f)]
[TestCase(0.01f)]
[TestCase(float.PositiveInfinity)]
[TestCase(float.NegativeInfinity)]
[TestCase(float.NaN)]
public void TestSingle(float val)
{
var stream = new MemoryStream();
Primitives.WritePrimitive(stream, val);

stream.Position = 0;

Primitives.ReadPrimitive(new ByteStream(stream), out float read);
Assert.That(read, Is.EqualTo(val));
}

[Test]
[TestCase(0)]
[TestCase(1)]
[TestCase(123.4)]
[TestCase(0.01)]
[TestCase(float.PositiveInfinity)]
[TestCase(float.NegativeInfinity)]
[TestCase(float.NaN)]
public void TestDouble(double val)
{
var stream = new MemoryStream();
Primitives.WritePrimitive(stream, val);

stream.Position = 0;

Primitives.ReadPrimitive(new ByteStream(stream), out double read);
Assert.That(read, Is.EqualTo(val));
}

// Stream wrapper that only reads one byte at a time to test the reading code.
private sealed class ByteStream : Stream
{
private readonly Stream _parent;

public ByteStream(Stream parent)
{
_parent = parent;
}

public override void Flush()
{
_parent.Flush();
}

public override long Seek(long offset, SeekOrigin origin)
{
return _parent.Seek(offset, origin);
}

public override void SetLength(long value)
{
_parent.SetLength(value);
}

public override int Read(byte[] buffer, int offset, int count)
{
return _parent.Read(buffer, offset, 1);
}

public override void Write(byte[] buffer, int offset, int count)
{
_parent.Write(buffer, offset, count);
}

public override bool CanRead => _parent.CanRead;

public override bool CanSeek => _parent.CanSeek;

public override bool CanWrite => _parent.CanWrite;

public override long Length => _parent.Length;

public override long Position
{
get => _parent.Position;
set => _parent.Position = value;
}
}
}
}
6 changes: 6 additions & 0 deletions NetSerializer.sln
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Test", "Test\Test.csproj",
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PrimitiveTest", "PrimitiveTest\PrimitiveTest.csproj", "{CBA6D818-4B6A-4A80-95D5-7F5EC3FBB3C4}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NetSerializer.UnitTests", "NetSerializer.UnitTests\NetSerializer.UnitTests.csproj", "{17DC557B-DB9E-464C-A311-5AB83E5684AC}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -27,6 +29,10 @@ Global
{CBA6D818-4B6A-4A80-95D5-7F5EC3FBB3C4}.Debug|Any CPU.Build.0 = Debug|Any CPU
{CBA6D818-4B6A-4A80-95D5-7F5EC3FBB3C4}.Release|Any CPU.ActiveCfg = Release|Any CPU
{CBA6D818-4B6A-4A80-95D5-7F5EC3FBB3C4}.Release|Any CPU.Build.0 = Release|Any CPU
{17DC557B-DB9E-464C-A311-5AB83E5684AC}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{17DC557B-DB9E-464C-A311-5AB83E5684AC}.Debug|Any CPU.Build.0 = Debug|Any CPU
{17DC557B-DB9E-464C-A311-5AB83E5684AC}.Release|Any CPU.ActiveCfg = Release|Any CPU
{17DC557B-DB9E-464C-A311-5AB83E5684AC}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down
2 changes: 1 addition & 1 deletion NetSerializer/NetSerializer.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.1;net45;netcoreapp3.0</TargetFrameworks>
<TargetFrameworks>netstandard2.1;net45;netcoreapp3.1;net5.0</TargetFrameworks>
<Description>A very fast and minimal serializer</Description>
<Company>Tomi Valkeinen</Company>
<Copyright>Copyright © 2012-2019 Tomi Valkeinen</Copyright>
Expand Down
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
2 changes: 1 addition & 1 deletion PrimitiveTest/PrimitiveTest.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netcoreapp3.0;net45</TargetFrameworks>
<TargetFrameworks>netcoreapp3.1;net45;net5.0</TargetFrameworks>
<OutputType>Exe</OutputType>
</PropertyGroup>

Expand Down
2 changes: 1 addition & 1 deletion Test/Test.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netcoreapp3.0;net45</TargetFrameworks>
<TargetFrameworks>netcoreapp3.1;net45;net5.0</TargetFrameworks>
<OutputType>Exe</OutputType>
</PropertyGroup>

Expand Down