From 4cd1709a386bfc10f5d088d7ecf447965f32907c Mon Sep 17 00:00:00 2001 From: Brant Burnett Date: Thu, 16 May 2024 19:36:38 -0400 Subject: [PATCH] Redesign LeftShiftOverflows test 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. --- Snappier.Benchmarks/LeftShiftOverflows.cs | 17 ++++++++++++ Snappier.Tests/HelpersTests.cs | 34 ++++++++++++++++++++++- Snappier/Internal/Helpers.cs | 15 ++++------ 3 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 Snappier.Benchmarks/LeftShiftOverflows.cs diff --git a/Snappier.Benchmarks/LeftShiftOverflows.cs b/Snappier.Benchmarks/LeftShiftOverflows.cs new file mode 100644 index 0000000..b0151fd --- /dev/null +++ b/Snappier.Benchmarks/LeftShiftOverflows.cs @@ -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); + } + } +} diff --git a/Snappier.Tests/HelpersTests.cs b/Snappier.Tests/HelpersTests.cs index 66af6dd..d11ed5b 100644 --- a/Snappier.Tests/HelpersTests.cs +++ b/Snappier.Tests/HelpersTests.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using Snappier.Internal; using Xunit; @@ -9,6 +8,39 @@ 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 Log2FloorNonZeroValues() => diff --git a/Snappier/Internal/Helpers.cs b/Snappier/Internal/Helpers.cs index 2118a1a..c3a58d8 100644 --- a/Snappier/Internal/Helpers.cs +++ b/Snappier/Internal/Helpers.cs @@ -45,17 +45,12 @@ public static int MaxCompressedLength(int sourceBytes) return 32 + sourceBytes + sourceBytes / 6 + 1; } - private static ReadOnlySpan 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)