Skip to content

Commit

Permalink
Use throwhelper to increase change of inlining
Browse files Browse the repository at this point in the history
- Add a ThrowHelper utility class that provides non-returning methods
  for throwing exceptions, this in turn helps to reduce the code size,
  there-by increasing the chances of inlining simple (getter/setter)
  methods, which has cascading CQ improvments in its own.
  (See: dotnet/coreclr#6103)
- Convert checks in the form of `(x < 0 || x > max)` to `(uint) x > max`
  to further reduce code-size and increase inlining chances
- Update MSTest package deps
- Drop netfx.props (which I introduces in a previous PR to support
  building on linux) and replace with the now standard/canonical way of
  referencing NetFX BCL on all platforms with a <PackageReference>
- closes real-logic#588
  • Loading branch information
damageboy committed Jul 16, 2019
1 parent ad5a1cf commit 217a07c
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 47 deletions.
26 changes: 0 additions & 26 deletions csharp/netfx.props

This file was deleted.

47 changes: 47 additions & 0 deletions csharp/sbe-dll/ThrowHelper.cs
@@ -0,0 +1,47 @@
using System;

namespace Org.SbeTool.Sbe.Dll
{
/// <summary>
/// Helper class that provides non-returning methods that throw common exception
/// from the generated C# code
/// </summary>
public class ThrowHelper
{
/// <summary>
/// Throws a <see cref="ArgumentOutOfRangeException"/> when the "count" parameter is out of range
/// </summary>
/// <param name="count">the parameter that triggered the exception</param>
public static void ThrowCountOutOfRangeException(int count) =>
throw new ArgumentOutOfRangeException("count", $"Outside allowed range: count={count}");

/// <summary>
/// Throws a <see cref="InvalidOperationException"/> when an invalid operation is invoked on a message (for example: enumerating a group past it's maximal count)
/// </summary>
public static void ThrowInvalidOperationException() =>
throw new InvalidOperationException();

/// <summary>
/// Throws a <see cref="IndexOutOfRangeException" /> when the "index" parameter is out of range
/// </summary>
/// <param name="index">the parameter that triggered the exception</param>
public static void ThrowIndexOutOfRangeException(int index) =>
throw new IndexOutOfRangeException($"index out of range: index={index}");

/// <summary>
/// Throws a <see cref="ArgumentOutOfRangeException"/> when a too-small <see cref="Span{T}"/>
/// is provided to a getter
/// </summary>
/// <param name="length">The length of the too-small Span</param>
public static void ThrowWhenSpanLengthTooSmall(int length) =>
throw new ArgumentOutOfRangeException("dst", $"dst.Length={length} is too small.");

/// <summary>
/// Throws a <see cref="ArgumentOutOfRangeException"/> when a too-large <see cref="Span{T}"/>
/// is provided to a setter
/// </summary>
/// <param name="length">The length of the too-large Span</param>
public static void ThrowWhenSpanLengthTooLarge(int length) =>
throw new ArgumentOutOfRangeException("src", $"src.Length={length} is too large.");
}
}
5 changes: 2 additions & 3 deletions csharp/sbe-dll/sbe-dll.csproj
@@ -1,6 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\netfx.props" />

<PropertyGroup>
<TargetFrameworks>net45;netstandard2.0</TargetFrameworks>
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
Expand All @@ -23,7 +21,8 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Memory" Version="4.5.1" />
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0-preview.2" PrivateAssets="All" />
<PackageReference Include="System.Memory" Version="4.5.3" />
</ItemGroup>

</Project>
9 changes: 4 additions & 5 deletions csharp/sbe-tests/sbe-tests.csproj
@@ -1,6 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\netfx.props" />

<PropertyGroup>
<TargetFrameworks>net45;netcoreapp2.1</TargetFrameworks>
<RootNamespace>Org.SbeTool.Sbe.Tests</RootNamespace>
Expand All @@ -25,9 +23,10 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.6.1" />
<PackageReference Include="MSTest.TestAdapter" Version="1.2.0" />
<PackageReference Include="MSTest.TestFramework" Version="1.2.0" />
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0-preview.2" PrivateAssets="All" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.2.0" />
<PackageReference Include="MSTest.TestAdapter" Version="1.4.0" />
<PackageReference Include="MSTest.TestFramework" Version="1.4.0" />
</ItemGroup>

<ItemGroup>
Expand Down
Expand Up @@ -210,14 +210,16 @@ private void generateGroupClassHeader(
final String typeForBlockLength = cSharpTypeName(tokens.get(index + 2).encoding().primitiveType());
final String typeForNumInGroup = cSharpTypeName(numInGroupToken.encoding().primitiveType());

final String throwCondition = numInGroupToken.encoding().applicableMinValue().longValue() == 0 ?
"if ((uint) count > %3$d)\n" :
"if (count < %2$d || count > %3$d)\n";

sb.append(String.format("\n" +
indent + INDENT + "public void WrapForEncode(%1$s parentMessage, DirectBuffer buffer, int count)\n" +
indent + INDENT + "{\n" +
indent + INDENT + INDENT + "if (count < %2$d || count > %3$d)\n" +
indent + INDENT + INDENT + throwCondition +
indent + INDENT + INDENT + "{\n" +
indent + INDENT + INDENT + INDENT + "throw new ArgumentOutOfRangeException(\"count\",\n" +
indent + INDENT + INDENT + INDENT + INDENT + "\"Outside allowed range: count=\" + count +\n" +
indent + INDENT + INDENT + INDENT + INDENT + "\", min=%2$d, max=%3$d\");\n" +
indent + INDENT + INDENT + INDENT + "ThrowHelper.ThrowCountOutOfRangeException(count);\n" +
indent + INDENT + INDENT + "}\n\n" +
indent + INDENT + INDENT + "_parentMessage = parentMessage;\n" +
indent + INDENT + INDENT + "_buffer = buffer;\n" +
Expand Down Expand Up @@ -258,7 +260,7 @@ private void generateGroupEnumerator(final StringBuilder sb, final String groupN
indent + INDENT + "{\n" +
indent + INDENT + INDENT + "if (_index + 1 >= _count)\n" +
indent + INDENT + INDENT + "{\n" +
indent + INDENT + INDENT + INDENT + "throw new InvalidOperationException();\n" +
indent + INDENT + INDENT + INDENT + "ThrowHelper.ThrowInvalidOperationException();\n" +
indent + INDENT + INDENT + "}\n\n" +
indent + INDENT + INDENT + "_offset = _parentMessage.Limit;\n" +
indent + INDENT + INDENT + "_parentMessage.Limit = _offset + _blockLength;\n" +
Expand Down Expand Up @@ -811,9 +813,9 @@ private CharSequence generateArrayProperty(
sb.append(String.format("\n" +
indent + "public %1$s Get%2$s(int index)\n" +
indent + "{\n" +
indent + INDENT + "if (index < 0 || index >= %3$d)\n" +
indent + INDENT + "if ((uint) index >= %3$d)\n" +
indent + INDENT + "{\n" +
indent + INDENT + INDENT + "throw new IndexOutOfRangeException(\"index out of range: index=\" + index);\n" +
indent + INDENT + INDENT + "ThrowHelper.ThrowIndexOutOfRangeException(index);\n" +
indent + INDENT + "}\n\n" +
"%4$s" +
indent + INDENT + "return _buffer.%5$sGet%8$s(_offset + %6$d + (index * %7$d));\n" +
Expand All @@ -825,9 +827,9 @@ private CharSequence generateArrayProperty(
sb.append(String.format("\n" +
indent + "public void Set%1$s(int index, %2$s value)\n" +
indent + "{\n" +
indent + INDENT + "if (index < 0 || index >= %3$d)\n" +
indent + INDENT + "if ((uint) index >= %3$d)\n" +
indent + INDENT + "{\n" +
indent + INDENT + INDENT + "throw new IndexOutOfRangeException(\"index out of range: index=\" + index);\n" +
indent + INDENT + INDENT + "ThrowHelper.ThrowIndexOutOfRangeException(index);\n" +
indent + INDENT + "}\n\n" +
indent + INDENT + "_buffer.%4$sPut%7$s(_offset + %5$d + (index * %6$d), value);\n" +
indent + "}\n",
Expand All @@ -852,8 +854,7 @@ private CharSequence generateArrayProperty(
indent + INDENT + "const int length = %2$d;\n" +
indent + INDENT + "if (dst.Length < length)\n" +
indent + INDENT + "{\n" +
indent + INDENT + INDENT +
"throw new ArgumentOutOfRangeException($\"dst.Length={dst.Length} is too large.\");\n" +
indent + INDENT + INDENT + "ThrowHelper.ThrowWhenSpanLengthTooSmall(dst.Length);\n" +
indent + INDENT + "}\n\n" +
"%3$s" +
indent + INDENT + "_buffer.GetBytes(_offset + %4$d, dst);\n" +
Expand All @@ -874,8 +875,7 @@ private CharSequence generateArrayProperty(
indent + INDENT + "const int length = %2$d;\n" +
indent + INDENT + "if (src.Length > length)\n" +
indent + INDENT + "{\n" +
indent + INDENT + INDENT +
"throw new ArgumentOutOfRangeException($\"src.Length={src.Length} is too large.\");\n" +
indent + INDENT + INDENT + "ThrowHelper.ThrowWhenSpanLengthTooLarge(src.Length);\n" +
indent + INDENT + "}\n\n" +
indent + INDENT + "_buffer.SetBytes(_offset + %3$d, src);\n" +
indent + "}\n",
Expand Down

0 comments on commit 217a07c

Please sign in to comment.