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.

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

Jump to bottom

Add .NET 5 target and improve WriteString performance with SIMD #8147

Merged
merged 4 commits into from Feb 16, 2021
Merged

Add .NET 5 target and improve WriteString performance with SIMD #8147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8932446
Improve WriteString perf with SIMD
JamesNK Dec 15, 2020
d9889a4
Fix 5.0 SDK Linux restore issue
JamesNK Feb 3, 2021
818c2ba
add libicu63 to C# dockerfile
jtattermusch Feb 15, 2021
e4cd821
Add comments
JamesNK Feb 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion csharp/install_dotnet_sdk.ps1
View file
Open in desktop
Expand Up @@ -17,4 +17,4 @@ Invoke-WebRequest -Uri $InstallScriptUrl -OutFile $InstallScriptPath
# The SDK versions to install should be kept in sync with versions
# installed by kokoro/linux/dockerfile/test/csharp/Dockerfile
&$InstallScriptPath -Version 2.1.802
&$InstallScriptPath -Version 3.1.301
&$InstallScriptPath -Version 5.0.102
2 changes: 1 addition & 1 deletion csharp/src/Google.Protobuf.Benchmarks/Google.Protobuf.Benchmarks.csproj
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>net5.0</TargetFramework>
<AssemblyOriginatorKeyFile>../../keys/Google.Protobuf.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<IsPackable>False</IsPackable>
Expand Down
12 changes: 6 additions & 6 deletions csharp/src/Google.Protobuf.Test/Buffers/ArrayBufferWriter.cs
View file
Open in desktop
Expand Up @@ -42,18 +42,18 @@ namespace Google.Protobuf.Buffers
/// ArrayBufferWriter is originally from corefx, and has been contributed to Protobuf
/// https://github.com/dotnet/runtime/blob/071da4c41aa808c949a773b92dca6f88de9d11f3/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
/// </summary>
internal sealed class ArrayBufferWriter<T> : IBufferWriter<T>
internal sealed class TestArrayBufferWriter<T> : IBufferWriter<T>
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
{
private T[] _buffer;
private int _index;

private const int DefaultInitialBufferSize = 256;

/// <summary>
/// Creates an instance of an <see cref="ArrayBufferWriter{T}"/>, in which data can be written to,
/// Creates an instance of an <see cref="TestArrayBufferWriter{T}"/>, in which data can be written to,
/// with the default initial capacity.
/// </summary>
public ArrayBufferWriter()
public TestArrayBufferWriter()
{
_buffer = new T[0];
_index = 0;
Expand All @@ -66,14 +66,14 @@ public ArrayBufferWriter()
public int? MaxGrowBy { get; set; }

/// <summary>
/// Creates an instance of an <see cref="ArrayBufferWriter{T}"/>, in which data can be written to,
/// Creates an instance of an <see cref="TestArrayBufferWriter{T}"/>, in which data can be written to,
/// with an initial capacity specified.
/// </summary>
/// <param name="initialCapacity">The minimum capacity with which to initialize the underlying buffer.</param>
/// <exception cref="ArgumentException">
/// Thrown when <paramref name="initialCapacity"/> is not positive (i.e. less than or equal to 0).
/// </exception>
public ArrayBufferWriter(int initialCapacity)
public TestArrayBufferWriter(int initialCapacity)
{
if (initialCapacity <= 0)
throw new ArgumentException(nameof(initialCapacity));
Expand Down Expand Up @@ -111,7 +111,7 @@ public ArrayBufferWriter(int initialCapacity)
/// Clears the data written to the underlying buffer.
/// </summary>
/// <remarks>
/// You must clear the <see cref="ArrayBufferWriter{T}"/> before trying to re-use it.
/// You must clear the <see cref="TestArrayBufferWriter{T}"/> before trying to re-use it.
/// </remarks>
public void Clear()
{
Expand Down
61 changes: 50 additions & 11 deletions csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs
View file
Open in desktop
Expand Up @@ -58,7 +58,7 @@ private static void AssertWriteVarint(byte[] data, ulong value)
Assert.AreEqual(data, rawOutput.ToArray());

// IBufferWriter
var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteUInt32((uint) value);
ctx.Flush();
Expand All @@ -77,7 +77,7 @@ private static void AssertWriteVarint(byte[] data, ulong value)
Assert.AreEqual(data, rawOutput.ToArray());

// IBufferWriter
var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteUInt64(value);
ctx.Flush();
Expand All @@ -100,7 +100,7 @@ private static void AssertWriteVarint(byte[] data, ulong value)
output.Flush();
Assert.AreEqual(data, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
bufferWriter.MaxGrowBy = bufferSize;
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteUInt32((uint) value);
Expand All @@ -115,7 +115,7 @@ private static void AssertWriteVarint(byte[] data, ulong value)
output.Flush();
Assert.AreEqual(data, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
bufferWriter.MaxGrowBy = bufferSize;
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteUInt64(value);
Expand Down Expand Up @@ -174,7 +174,7 @@ private static void AssertWriteLittleEndian32(byte[] data, uint value)
output.Flush();
Assert.AreEqual(data, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteFixed32(value);
ctx.Flush();
Expand All @@ -190,7 +190,7 @@ private static void AssertWriteLittleEndian32(byte[] data, uint value)
output.Flush();
Assert.AreEqual(data, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
bufferWriter.MaxGrowBy = bufferSize;
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteFixed32(value);
Expand All @@ -212,7 +212,7 @@ private static void AssertWriteLittleEndian64(byte[] data, ulong value)
output.Flush();
Assert.AreEqual(data, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteFixed64(value);
ctx.Flush();
Expand All @@ -228,7 +228,7 @@ private static void AssertWriteLittleEndian64(byte[] data, ulong value)
output.Flush();
Assert.AreEqual(data, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
bufferWriter.MaxGrowBy = blockSize;
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteFixed64(value);
Expand Down Expand Up @@ -270,7 +270,7 @@ public void WriteWholeMessage_VaryingBlockSizes()
output.Flush();
Assert.AreEqual(rawBytes, rawOutput.ToArray());

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
bufferWriter.MaxGrowBy = blockSize;
message.WriteTo(bufferWriter);
Assert.AreEqual(rawBytes, bufferWriter.WrittenSpan.ToArray());
Expand All @@ -292,7 +292,7 @@ public void WriteContext_WritesWithFlushes()
output.Flush();
byte[] expectedBytes2 = expectedOutput.ToArray();

var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
WriteContext.Initialize(bufferWriter, out WriteContext ctx);
ctx.WriteMessage(message);
ctx.Flush();
Expand Down Expand Up @@ -519,7 +519,21 @@ public void Dispose_FromByteArray()
}

[Test]
public void WriteStringsOfDifferentSizes()
public void WriteString_AsciiSmall_MaxUtf8SizeExceedsBuffer()
{
var buffer = new byte[5];
var output = new CodedOutputStream(buffer);
output.WriteString("ABC");

output.Flush();

// Verify written content
var input = new CodedInputStream(buffer);
Assert.AreEqual("ABC", input.ReadString());
}

[Test]
public void WriteStringsOfDifferentSizes_Ascii()
{
for (int i = 1; i <= 1024; i++)
{
Expand All @@ -540,5 +554,30 @@ public void WriteStringsOfDifferentSizes()
Assert.AreEqual(s, input.ReadString());
}
}

[Test]
public void WriteStringsOfDifferentSizes_Unicode()
{
for (int i = 1; i <= 1024; i++)
{
var buffer = new byte[4096];
var output = new CodedOutputStream(buffer);
var sb = new StringBuilder();
for (int j = 0; j < i; j++)
{
char c = (char)((j % 10) + 10112);
sb.Append(c.ToString()); // incrementing unicode numbers, repeating
}
var s = sb.ToString();
output.WriteString(s);

output.Flush();

// Verify written content
var input = new CodedInputStream(buffer);

Assert.AreEqual(s, input.ReadString());
}
}
}
}
2 changes: 1 addition & 1 deletion csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj
View file
Open in desktop
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net451;netcoreapp2.1</TargetFrameworks>
<TargetFrameworks>net451;netcoreapp2.1;net50</TargetFrameworks>
<AssemblyOriginatorKeyFile>../../keys/Google.Protobuf.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
<IsPackable>False</IsPackable>
Expand Down
4 changes: 4 additions & 0 deletions csharp/src/Google.Protobuf.Test/JsonParserTest.cs
View file
Open in desktop
Expand Up @@ -551,9 +551,13 @@ public void NumberToDouble_Valid(string jsonValue, double expectedParsedValue)
}

[Test]
// Skip these test cases in .NET 5 because floating point parsing supports bigger values.
// These big values won't throw an error in the test.
#if !NET5_0
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
[TestCase("1.7977e308")]
[TestCase("-1.7977e308")]
[TestCase("1e309")]
#endif
[TestCase("1,0")]
[TestCase("1.0.0")]
[TestCase("+1")]
Expand Down
4 changes: 4 additions & 0 deletions csharp/src/Google.Protobuf.Test/JsonTokenizerTest.cs
View file
Open in desktop
Expand Up @@ -199,8 +199,12 @@ public void NumberValue(string json, double expectedValue)
[TestCase("1e-")]
[TestCase("--")]
[TestCase("--1")]
// Skip these test cases in .NET 5 because floating point parsing supports bigger values.
// These big values won't throw an error in the test.
#if !NET5_0
[TestCase("-1.7977e308")]
[TestCase("1.7977e308")]
#endif
public void InvalidNumberValue(string json)
{
AssertThrowsAfter(json);
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Google.Protobuf.Test/LegacyGeneratedCodeTest.cs
View file
Open in desktop
Expand Up @@ -141,7 +141,7 @@ public void LegacyGeneratedCodeThrowsWithIBufferWriter()
};
var exception = Assert.Throws<InvalidProtocolBufferException>(() =>
{
WriteContext.Initialize(new ArrayBufferWriter<byte>(), out WriteContext writeCtx);
WriteContext.Initialize(new TestArrayBufferWriter<byte>(), out WriteContext writeCtx);
((IBufferMessage)message).InternalWriteTo(ref writeCtx);
});
Assert.AreEqual($"Message {typeof(LegacyGeneratedCodeMessageA).Name} doesn't provide the generated method that enables WriteContext-based serialization. You might need to regenerate the generated protobuf code.", exception.Message);
Expand Down
6 changes: 3 additions & 3 deletions csharp/src/Google.Protobuf.Test/MessageParsingHelpers.cs
View file
Open in desktop
Expand Up @@ -83,7 +83,7 @@ public static void AssertReadingMessage(MessageParser parser, byte[] bytes, Acti
var bytes = message.ToByteArray();

// also serialize using IBufferWriter and check it leads to the same data
var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
message.WriteTo(bufferWriter);
Assert.AreEqual(bytes, bufferWriter.WrittenSpan.ToArray(), "Both serialization approaches need to result in the same data.");

Expand Down Expand Up @@ -112,7 +112,7 @@ public static void AssertWritingMessage(IMessage message)
Assert.AreEqual(message.CalculateSize(), bytes.Length);

// serialize using IBufferWriter and check it leads to the same output
var bufferWriter = new ArrayBufferWriter<byte>();
var bufferWriter = new TestArrayBufferWriter<byte>();
message.WriteTo(bufferWriter);
Assert.AreEqual(bytes, bufferWriter.WrittenSpan.ToArray());

Expand All @@ -124,7 +124,7 @@ public static void AssertWritingMessage(IMessage message)
// test for different IBufferWriter.GetSpan() segment sizes
for (int blockSize = 1; blockSize < 256; blockSize *= 2)
{
var segmentedBufferWriter = new ArrayBufferWriter<byte>();
var segmentedBufferWriter = new TestArrayBufferWriter<byte>();
segmentedBufferWriter.MaxGrowBy = blockSize;
message.WriteTo(segmentedBufferWriter);
Assert.AreEqual(bytes, segmentedBufferWriter.WrittenSpan.ToArray());
Expand Down
16 changes: 12 additions & 4 deletions csharp/src/Google.Protobuf/Google.Protobuf.csproj
View file
Open in desktop
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Description>C# runtime library for Protocol Buffers - Google's data interchange format.</Description>
Expand All @@ -8,7 +8,7 @@
<!-- C# 7.2 is required for Span/BufferWriter/ReadOnlySequence -->
<LangVersion>7.2</LangVersion>
<Authors>Google Inc.</Authors>
<TargetFrameworks>netstandard1.1;netstandard2.0;net45</TargetFrameworks>
<TargetFrameworks>netstandard1.1;netstandard2.0;net45;net50</TargetFrameworks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<AssemblyOriginatorKeyFile>../../keys/Google.Protobuf.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
Expand All @@ -27,15 +27,23 @@
<DefineConstants>$(DefineConstants);GOOGLE_PROTOBUF_SUPPORT_FAST_STRING</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'net50' ">
<DefineConstants>$(DefineConstants);GOOGLE_PROTOBUF_SUPPORT_FAST_STRING;GOOGLE_PROTOBUF_SIMD</DefineConstants>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="System.Memory" Version="4.5.3"/>
<PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" Version="1.0.0"/>
<!-- Needed for the net45 build to work on Unix. See https://github.com/dotnet/designs/pull/33 -->
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" PrivateAssets="All" Version="1.0.0"/>
</ItemGroup>

<!-- Needed for netcoreapp2.1 to work correctly. .NET is not able to load the assembly without this -->
<ItemGroup Condition=" '$(TargetFramework)' == 'net45' OR '$(TargetFramework)' == 'netstandard1.1' ">
<PackageReference Include="System.Memory" Version="4.5.3"/>
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
<PackageReference Include="System.Memory" Version="4.5.3"/>
<!-- Needed for netcoreapp2.1 to work correctly. .NET is not able to load the assembly without this -->
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.5.2"/>
</ItemGroup>

Expand Down