Updated MemoryDiagnoserTests.

Small code cleanup.
dotnet · Apr 23, 2024 · 917988c · 917988c
1 parent ce134ab
commit 917988c
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 24 deletions.
diff --git a/src/BenchmarkDotNet/Engines/Engine.cs b/src/BenchmarkDotNet/Engines/Engine.cs
@@ -228,7 +228,7 @@ private ClockSpan Measure(Action<long> action, long invokeCount)
             if (RuntimeInformation.IsNetCore && Environment.Version.Major is >= 3 and <= 6 && Environment.GetEnvironmentVariable("COMPlus_TieredCompilation") != "0")
             {
                 // #1542
-                // We put the current thread to sleep so tiered jit can kick in, compile it's stuff
+                // We put the current thread to sleep so tiered jit can kick in, compile its stuff,
                 // and NOT allocate anything on the background thread when we are measuring allocations.
                 // This is only an issue on netcoreapp3.0 to net6.0. Tiered jit allocations were fixed in net7.0,
                 // and netcoreapp2.X uses only GetAllocatedBytesForCurrentThread which doesn't capture the tiered jit allocations.
@@ -245,10 +245,9 @@ private ClockSpan Measure(Action<long> action, long invokeCount)
             IterationCleanupAction(); // we run iteration cleanup after collecting GC stats
 
             var totalOperationsCount = data.InvokeCount * OperationsPerInvoke;
-            gcStats = gcStats.WithTotalOperations(totalOperationsCount);
-            ThreadingStats threadingStats = (finalThreadingStats - initialThreadingStats).WithTotalOperations(data.InvokeCount * OperationsPerInvoke);
-
-            return (gcStats, threadingStats, exceptionsStats.ExceptionsCount / (double)totalOperationsCount);
+            return (gcStats.WithTotalOperations(totalOperationsCount),
+                (finalThreadingStats - initialThreadingStats).WithTotalOperations(totalOperationsCount),
+                exceptionsStats.ExceptionsCount / (double)totalOperationsCount);
         }
 
         // Isolate the allocation measurement and skip tier0 jit to make sure we don't get any unexpected allocations.

diff --git a/tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs b/tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs
@@ -35,7 +35,11 @@ public class MemoryDiagnoserTests
         public static IEnumerable<object[]> GetToolchains()
         {
             yield return new object[] { Job.Default.GetToolchain() };
-            yield return new object[] { InProcessEmitToolchain.Instance };
+            // InProcessEmit reports flaky allocations in current .Net 8.
+            if (!RuntimeInformation.IsNetCore)
+            {
+                yield return new object[] { InProcessEmitToolchain.Instance };
+            }
         }
 
         public class AccurateAllocations
@@ -104,7 +108,7 @@ private void AllocateUntilGcWakesUp()
             }
         }
 
-        [Theory(Skip = "#1542 Tiered JIT Thread allocates memory in the background"), MemberData(nameof(GetToolchains))]
+        [Theory, MemberData(nameof(GetToolchains))]
         [Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
         public void MemoryDiagnoserDoesNotIncludeAllocationsFromSetupAndCleanup(IToolchain toolchain)
         {
@@ -117,23 +121,41 @@ public void MemoryDiagnoserDoesNotIncludeAllocationsFromSetupAndCleanup(IToolcha
         public class NoAllocationsAtAll
         {
             [Benchmark] public void EmptyMethod() { }
+
+            [Benchmark]
+            public ulong TimeConsuming()
+            {
+                var r = 1ul;
+                for (var i = 0; i < 50_000_000; i++)
+                {
+                    r /= 1;
+                }
+                return r;
+            }
         }
 
         [Theory, MemberData(nameof(GetToolchains))]
         [Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
         public void EngineShouldNotInterfereAllocationResults(IToolchain toolchain)
         {
-            if (RuntimeInformation.IsFullFramework && toolchain.IsInProcess)
-            {
-                return; // this test is flaky on Full Framework
-            }
-
             AssertAllocations(toolchain, typeof(NoAllocationsAtAll), new Dictionary<string, long>
             {
                 { nameof(NoAllocationsAtAll.EmptyMethod), 0 }
             });
         }
 
+        // #1542
+        [Theory, MemberData(nameof(GetToolchains))]
+        [Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
+        public void TieredJitShouldNotInterfereAllocationResults(IToolchain toolchain)
+        {
+            AssertAllocations(toolchain, typeof(NoAllocationsAtAll), new Dictionary<string, long>
+            {
+                { nameof(NoAllocationsAtAll.TimeConsuming), 0 }
+            },
+            iterationCount: 10); // 1 iteration is not enough to repro the problem
+        }
+
         public class NoBoxing
         {
             [Benchmark] public ValueTuple<int> ReturnsValueType() => new ValueTuple<int>(0);
@@ -216,8 +238,7 @@ public byte[] SixtyFourBytesArray()
             }
         }
 
-        [Theory(Skip = "#1542 Tiered JIT Thread allocates memory in the background"), MemberData(nameof(GetToolchains))]
-        //[TheoryNetCoreOnly("Only .NET Core 2.0+ API is bug free for this case"), MemberData(nameof(GetToolchains))]
+        [TheoryEnvSpecific("Full Framework cannot measure precisely enough for low invocation counts.", EnvRequirement.DotNetCoreOnly), MemberData(nameof(GetToolchains))]
         [Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
         public void AllocationQuantumIsNotAnIssueForNetCore21Plus(IToolchain toolchain)
         {
@@ -256,8 +277,7 @@ public void Allocate()
             }
         }
 
-        [TheoryEnvSpecific(".NET Core 3.0 preview6+ exposes a GC.GetTotalAllocatedBytes method which makes it possible to work",
-            EnvRequirement.DotNetCore30Only)]
+        [Theory(Skip = "Test is currently failing on all toolchains.")]
         [MemberData(nameof(GetToolchains))]
         [Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
         public void MemoryDiagnoserIsAccurateForMultiThreadedBenchmarks(IToolchain toolchain)
@@ -274,9 +294,9 @@ public void MemoryDiagnoserIsAccurateForMultiThreadedBenchmarks(IToolchain toolc
             });
         }
 
-        private void AssertAllocations(IToolchain toolchain, Type benchmarkType, Dictionary<string, long> benchmarksAllocationsValidators)
+        private void AssertAllocations(IToolchain toolchain, Type benchmarkType, Dictionary<string, long> benchmarksAllocationsValidators, int iterationCount = 1)
         {
-            var config = CreateConfig(toolchain);
+            var config = CreateConfig(toolchain, iterationCount);
             var benchmarks = BenchmarkConverter.TypeToBenchmarks(benchmarkType, config);
 
             var summary = BenchmarkRunner.Run(benchmarks);
@@ -312,16 +332,15 @@ private void AssertAllocations(IToolchain toolchain, Type benchmarkType, Diction
             }
         }
 
-        private IConfig CreateConfig(IToolchain toolchain)
+        private IConfig CreateConfig(IToolchain toolchain, int iterationCount = 1) // Single iteration is enough for most of the tests.
             => ManualConfig.CreateEmpty()
                 .AddJob(Job.ShortRun
                     .WithEvaluateOverhead(false) // no need to run idle for this test
                     .WithWarmupCount(0) // don't run warmup to save some time for our CI runs
-                    .WithIterationCount(1) // single iteration is enough for us
+                    .WithIterationCount(iterationCount)
                     .WithGcForce(false)
                     .WithGcServer(false)
                     .WithGcConcurrent(false)
-                    .WithEnvironmentVariable("COMPlus_TieredCompilation", "0") // Tiered JIT can allocate some memory on a background thread, let's disable it to make our tests less flaky (#1542)
                     .WithToolchain(toolchain))
                 .AddColumnProvider(DefaultColumnProviders.Instance)
                 .AddDiagnoser(MemoryDiagnoser.Default)

diff --git a/tests/BenchmarkDotNet.Tests/XUnit/EnvRequirement.cs b/tests/BenchmarkDotNet.Tests/XUnit/EnvRequirement.cs
@@ -7,6 +7,5 @@ public enum EnvRequirement
     NonLinux,
     FullFrameworkOnly,
     NonFullFramework,
-    DotNetCoreOnly,
-    DotNetCore30Only
+    DotNetCoreOnly
 }
diff --git a/tests/BenchmarkDotNet.Tests/XUnit/EnvRequirementChecker.cs b/tests/BenchmarkDotNet.Tests/XUnit/EnvRequirementChecker.cs
@@ -19,7 +19,6 @@ public static class EnvRequirementChecker
         EnvRequirement.FullFrameworkOnly => BdnRuntimeInformation.IsFullFramework ? null : "Full .NET Framework-only test",
         EnvRequirement.NonFullFramework => !BdnRuntimeInformation.IsFullFramework ? null : "Non-Full .NET Framework test",
         EnvRequirement.DotNetCoreOnly => BdnRuntimeInformation.IsNetCore ? null : ".NET/.NET Core-only test",
-        EnvRequirement.DotNetCore30Only => IsRuntime(RuntimeMoniker.NetCoreApp30) ? null : ".NET Core 3.0-only test",
         _ => throw new ArgumentOutOfRangeException(nameof(requirement), requirement, "Unknown value")
     };