Remove an unnecessary branch from Log2Floor

Neither of the callsites using Helpers.Log2Floor ever pass a 0, so the special case to return -1 in the 0 case is unnecessary. BenchmarkDotNet v0.14.0, Windows 11 (10.0.26100.2314) Unknown processor .NET SDK 9.0.100 [Host] : .NET 9.0.0 (9.0.24.52809), X64 RyuJIT AVX2 Job-JLJFKR : .NET Framework 4.8.1 (4.8.9282.0), X64 RyuJIT VectorSize=256 Job-PXVNHY : .NET Framework 4.8.1 (4.8.9282.0), X64 RyuJIT VectorSize=256 Job-DVRAUN : .NET 6.0.36 (6.0.3624.51421), X64 RyuJIT AVX2 Job-GQEZIF : .NET 6.0.36 (6.0.3624.51421), X64 RyuJIT AVX2 Job-PUVQSP : .NET 8.0.11 (8.0.1124.51707), X64 RyuJIT AVX2 Job-YPVVHF : .NET 8.0.11 (8.0.1124.51707), X64 RyuJIT AVX2 Job-MKMULB : .NET 9.0.0 (9.0.24.52809), X64 RyuJIT AVX2 Job-PYOQFK : .NET 9.0.0 (9.0.24.52809), X64 RyuJIT AVX2 | Method | Runtime | BuildConfiguration | Mean | Error | StdDev | Median | Ratio | RatioSD | Rank | Code Size | |---------- |------------------- |------------------- |----------:|----------:|----------:|----------:|------:|--------:|-----:|----------:| | Log2Floor | .NET Framework 4.8 | Previous | 8.2904 ns | 0.1383 ns | 0.1155 ns | 8.2723 ns | 1.00 | 0.02 | 2 | 186 B | | Log2Floor | .NET Framework 4.8 | Default | 7.8699 ns | 0.0244 ns | 0.0204 ns | 7.8673 ns | 0.95 | 0.01 | 1 | 170 B | | | | | | | | | | | | | | Log2Floor | .NET 6.0 | Previous | 0.3634 ns | 0.0273 ns | 0.0256 ns | 0.3612 ns | 1.00 | 0.10 | 2 | 25 B | | Log2Floor | .NET 6.0 | Default | 0.0527 ns | 0.0175 ns | 0.0164 ns | 0.0534 ns | 0.15 | 0.05 | 1 | 14 B | | | | | | | | | | | | | | Log2Floor | .NET 8.0 | Previous | 0.1953 ns | 0.0041 ns | 0.0037 ns | 0.1947 ns | 1.00 | 0.03 | 2 | 25 B | | Log2Floor | .NET 8.0 | Default | 0.0030 ns | 0.0064 ns | 0.0053 ns | 0.0000 ns | 0.02 | 0.03 | 1 | 14 B | | | | | | | | | | | | | | Log2Floor | .NET 9.0 | Previous | 0.2028 ns | 0.0030 ns | 0.0028 ns | 0.2025 ns | 1.00 | 0.02 | 2 | 25 B | | Log2Floor | .NET 9.0 | Default | 0.0147 ns | 0.0064 ns | 0.0057 ns | 0.0127 ns | 0.07 | 0.03 | 1 | 14 B |
brantburnett · Nov 30, 2024 · 53dab59 · 53dab59
1 parent 74847fd
commit 53dab59
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 35 deletions.
diff --git a/Snappier.Benchmarks/Configuration/FrameworkCompareConfig.cs b/Snappier.Benchmarks/Configuration/FrameworkCompareConfig.cs
@@ -13,14 +13,15 @@ public FrameworkCompareConfig(Job baseJob)
                 .WithRuntime(ClrRuntime.Net48));
             AddJob(baseJob
                 .WithRuntime(CoreRuntime.Core60));
-
-            var job80 = baseJob.WithRuntime(CoreRuntime.Core80);
-            AddJob(job80.WithPgo(false));
-            AddJob(job80.WithPgo(true));
+            AddJob(baseJob
+                .WithRuntime(CoreRuntime.Core80)
+                .WithPgo(true));
+            AddJob(baseJob
+                .WithRuntime(CoreRuntime.Core90)
+                .WithPgo(true));
 
             AddLogicalGroupRules(BenchmarkLogicalGroupRule.ByJob);
 
-            AddColumn(PgoColumn.Default);
             HideColumns(Column.EnvironmentVariables);
         }
     }

diff --git a/Snappier.Benchmarks/Configuration/VersionComparisonConfig.cs b/Snappier.Benchmarks/Configuration/VersionComparisonConfig.cs
@@ -20,27 +20,26 @@ public VersionComparisonConfig(Job baseJob)
 
             var jobBefore48 = jobBefore.WithRuntime(ClrRuntime.Net48).AsBaseline();
             var jobBefore60 = jobBefore.WithRuntime(CoreRuntime.Core60).AsBaseline();
-            var jobBefore80 = jobBefore.WithRuntime(CoreRuntime.Core80).AsBaseline();
-            var jobBefore80Pgo = jobBefore80.WithPgo();
+            var jobBefore80 = jobBefore.WithRuntime(CoreRuntime.Core80).WithPgo().AsBaseline();
+            var jobBefore90 = jobBefore.WithRuntime(CoreRuntime.Core90).WithPgo().AsBaseline();
 
             var jobAfter48 = baseJob.WithRuntime(ClrRuntime.Net48);
             var jobAfter60 = baseJob.WithRuntime(CoreRuntime.Core60);
-            var jobAfter80 = baseJob.WithRuntime(CoreRuntime.Core80);
-            var jobAfter80Pgo = jobAfter80.WithPgo();
+            var jobAfter80 = baseJob.WithRuntime(CoreRuntime.Core80).WithPgo();
+            var jobAfter90 = baseJob.WithRuntime(CoreRuntime.Core90).WithPgo();
 
             AddJob(jobBefore48);
             AddJob(jobBefore60);
             AddJob(jobBefore80);
-            AddJob(jobBefore80Pgo);
+            AddJob(jobBefore90);
 
             AddJob(jobAfter48);
             AddJob(jobAfter60);
             AddJob(jobAfter80);
-            AddJob(jobAfter80Pgo);
+            AddJob(jobAfter90);
 
             WithOrderer(VersionComparisonOrderer.Default);
 
-            AddColumn(PgoColumn.Default);
             HideColumns(Column.EnvironmentVariables, Column.Job);
         }
 

diff --git a/Snappier.Tests/HelpersTests.cs b/Snappier.Tests/HelpersTests.cs
@@ -1,6 +1,4 @@
 using System;
-using System.Collections.Generic;
-using System.Linq;
 using Snappier.Internal;
 using Xunit;
 
@@ -39,26 +37,36 @@ public void LeftShiftOverflows_False(byte value, int shift)
             Assert.False(result);
         }
 
-        #endregion
-
-        #region Log2FloorNonZero
-
-        public static IEnumerable<object[]> Log2FloorNonZeroValues() =>
-            Enumerable.Range(1, 31).Select(p => new object[] {p});
+        public static TheoryData<uint> Log2FloorValues() =>
+        [
+            1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
+        ];
 
         [Theory]
-        [MemberData(nameof(Log2FloorNonZeroValues))]
-        public void Log2FloorNonZero(uint value)
+        [MemberData(nameof(Log2FloorValues))]
+        public void Log2Floor(uint value)
         {
             // Act
 
-            var result = Helpers.Log2FloorNonZero(value);
+            var result = Helpers.Log2Floor(value);
 
             // Assert
 
             Assert.Equal((int) Math.Floor(Math.Log(value, 2)), result);
         }
 
+        [Fact]
+        public void Log2Floor_Zero()
+        {
+            // Act
+
+            var result = Helpers.Log2Floor(0);
+
+            // Assert
+
+            Assert.Equal(0, result);
+        }
+
         #endregion
     }
 }
diff --git a/Snappier/Internal/Helpers.cs b/Snappier/Internal/Helpers.cs
@@ -165,21 +165,11 @@ ref MemoryMarshal.GetReference(Log2DeBruijn),
 #endif
 
         /// <summary>
-        /// Return floor(log2(n)) for positive integer n.  Returns -1 if n == 0.
+        /// Return floor(log2(n)) for positive integer n. Returns 0 for the special case n = 0.
         /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static int Log2Floor(uint n) =>
-            n == 0 ? -1 : Log2FloorNonZero(n);
-
-
-        /// <summary>
-        /// Return floor(log2(n)) for positive integer n.
-        /// </summary>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static int Log2FloorNonZero(uint n)
+        public static int Log2Floor(uint n)
         {
-            Debug.Assert(n != 0);
-
 #if NET6_0_OR_GREATER
             return BitOperations.Log2(n);
 #else