From 53dab59d4dd74b109b41197ecd2724422d112f53 Mon Sep 17 00:00:00 2001 From: Brant Burnett Date: Sat, 30 Nov 2024 11:41:13 -0500 Subject: [PATCH] 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 | --- .../Configuration/FrameworkCompareConfig.cs | 11 +++---- .../Configuration/VersionComparisonConfig.cs | 13 ++++---- Snappier.Tests/HelpersTests.cs | 30 ++++++++++++------- Snappier/Internal/Helpers.cs | 14 ++------- 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/Snappier.Benchmarks/Configuration/FrameworkCompareConfig.cs b/Snappier.Benchmarks/Configuration/FrameworkCompareConfig.cs index 939070d..67952c5 100644 --- 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 index 89b6cf9..84c912a 100644 --- 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 index d11ed5b..e45b7c1 100644 --- 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 Log2FloorNonZeroValues() => - Enumerable.Range(1, 31).Select(p => new object[] {p}); + public static TheoryData 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 index c3a58d8..9163cc7 100644 --- a/Snappier/Internal/Helpers.cs +++ b/Snappier/Internal/Helpers.cs @@ -165,21 +165,11 @@ ref MemoryMarshal.GetReference(Log2DeBruijn), #endif /// - /// 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. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int Log2Floor(uint n) => - n == 0 ? -1 : Log2FloorNonZero(n); - - - /// - /// Return floor(log2(n)) for positive integer n. - /// - [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