Skip to content

Commit

Permalink
Redesign LeftShiftOverflows test (#95)
Browse files Browse the repository at this point in the history
The current test was based on the C++ test, but the overhead of the
LeftShiftOverflowsMasks lookup is much higher on C#, especially on
legacy .NET Framework. In particular, this new approach avoids range
checks and theoretical exceptions.
  • Loading branch information
brantburnett authored May 16, 2024
1 parent f70627d commit 08094ad
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 11 deletions.
17 changes: 17 additions & 0 deletions Snappier.Benchmarks/LeftShiftOverflows.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using BenchmarkDotNet.Attributes;
using Snappier.Internal;

namespace Snappier.Benchmarks
{
public class LeftShiftOverflows
{
private byte value = 24;
private int shift = 7;

[Benchmark(Baseline = true)]
public bool Current()
{
return Helpers.LeftShiftOverflows(value, shift);
}
}
}
34 changes: 33 additions & 1 deletion Snappier.Tests/HelpersTests.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,46 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Snappier.Internal;
using Xunit;

namespace Snappier.Tests
{
public class HelpersTests
{
#region LeftShiftOverflows

[Theory]
[InlineData(2, 31)]
[InlineData(0xff, 25)]
public void LeftShiftOverflows_True(byte value, int shift)
{
// Act

var result = Helpers.LeftShiftOverflows(value, shift);

// Assert

Assert.True(result);
}

[Theory]
[InlineData(1, 31)]
[InlineData(0xff, 24)]
[InlineData(0, 31)]
public void LeftShiftOverflows_False(byte value, int shift)
{
// Act

var result = Helpers.LeftShiftOverflows(value, shift);

// Assert

Assert.False(result);
}

#endregion

#region Log2FloorNonZero

public static IEnumerable<object[]> Log2FloorNonZeroValues() =>
Expand Down
15 changes: 5 additions & 10 deletions Snappier/Internal/Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,12 @@ public static int MaxCompressedLength(int sourceBytes)
return 32 + sourceBytes + sourceBytes / 6 + 1;
}

private static ReadOnlySpan<byte> LeftShiftOverflowsMasks =>
[
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe
];

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool LeftShiftOverflows(byte value, int shift) =>
(value & LeftShiftOverflowsMasks[shift]) != 0;
public static bool LeftShiftOverflows(byte value, int shift)
{
Debug.Assert(shift < 32);
return (value & ~(0xffff_ffffu >>> shift)) != 0;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static uint ExtractLowBytes(uint value, int numBytes)
Expand Down

0 comments on commit 08094ad

Please sign in to comment.