Skip to content

Commit f6a4194

Browse files
committed
Re-enabled allocation tests.
Disable tiered jit by default in tests. Block finalizer thread during memory tests.
1 parent 63b62a9 commit f6a4194

File tree

2 files changed

+88
-23
lines changed

2 files changed

+88
-23
lines changed

src/BenchmarkDotNet/Engines/Engine.cs

+56-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,11 @@ private ClockSpan Measure(Action<long> action, long invokeCount)
240240

241241
// GC collect before measuring allocations.
242242
ForceGcCollect();
243-
var gcStats = MeasureWithGc(data.InvokeCount / data.UnrollFactor);
243+
GcStats gcStats;
244+
using (FinalizerBlocker.MaybeStart())
245+
{
246+
gcStats = MeasureWithGc(data.InvokeCount / data.UnrollFactor);
247+
}
244248

245249
exceptionsStats.Stop(); // this method might (de)allocate
246250
var finalThreadingStats = ThreadingStats.ReadFinal();
@@ -322,5 +326,56 @@ private static readonly Dictionary<string, HostSignal> MessagesToSignals
322326
public static bool TryGetSignal(string message, out HostSignal signal)
323327
=> MessagesToSignals.TryGetValue(message, out signal);
324328
}
329+
330+
// Very long key and value so this shouldn't be used outside of unit tests.
331+
internal const string UnitTestBlockFinalizerEnvKey = "BENCHMARKDOTNET_UNITTEST_BLOCK_FINALIZER_FOR_MEMORYDIAGNOSER";
332+
internal const string UnitTestBlockFinalizerEnvValue = UnitTestBlockFinalizerEnvKey + "_ACTIVE";
333+
334+
// To prevent finalizers interfering with allocation measurements for unit tests,
335+
// we block the finalizer thread until we've completed the measurement.
336+
// https://github.com/dotnet/runtime/issues/101536#issuecomment-2077647417
337+
private readonly struct FinalizerBlocker : IDisposable
338+
{
339+
private readonly ManualResetEventSlim hangEvent;
340+
341+
private FinalizerBlocker(ManualResetEventSlim hangEvent) => this.hangEvent = hangEvent;
342+
343+
private sealed class Impl
344+
{
345+
private readonly ManualResetEventSlim hangEvent = new (false);
346+
private readonly ManualResetEventSlim enteredFinalizerEvent = new (false);
347+
348+
~Impl()
349+
{
350+
enteredFinalizerEvent.Set();
351+
hangEvent.Wait();
352+
}
353+
354+
[MethodImpl(MethodImplOptions.NoInlining)]
355+
internal static (ManualResetEventSlim hangEvent, ManualResetEventSlim enteredFinalizerEvent) CreateWeakly()
356+
{
357+
var impl = new Impl();
358+
return (impl.hangEvent, impl.enteredFinalizerEvent);
359+
}
360+
}
361+
362+
internal static FinalizerBlocker MaybeStart()
363+
{
364+
if (Environment.GetEnvironmentVariable(UnitTestBlockFinalizerEnvKey) != UnitTestBlockFinalizerEnvValue)
365+
{
366+
return default;
367+
}
368+
var (hangEvent, enteredFinalizerEvent) = Impl.CreateWeakly();
369+
do
370+
{
371+
GC.Collect();
372+
// Do NOT call GC.WaitForPendingFinalizers.
373+
}
374+
while (!enteredFinalizerEvent.IsSet);
375+
return new FinalizerBlocker(hangEvent);
376+
}
377+
378+
public void Dispose() => hangEvent?.Set();
379+
}
325380
}
326381
}

tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs

+32-22
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ namespace BenchmarkDotNet.IntegrationTests
2828
{
2929
public class MemoryDiagnoserTests
3030
{
31-
// TODO: re-enable allocating tests after https://github.com/dotnet/runtime/issues/101536 is fixed.
32-
private const string AllocatingSkipReason = "GC collect causes allocations on finalizer thread";
33-
3431
private readonly ITestOutputHelper output;
3532

3633
public MemoryDiagnoserTests(ITestOutputHelper outputHelper) => output = outputHelper;
@@ -53,7 +50,7 @@ public class AccurateAllocations
5350
[Benchmark] public Task<int> AllocateTask() => Task.FromResult<int>(-12345);
5451
}
5552

56-
[Theory(Skip = AllocatingSkipReason), MemberData(nameof(GetToolchains))]
53+
[Theory, MemberData(nameof(GetToolchains))]
5754
[Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
5855
public void MemoryDiagnoserIsAccurate(IToolchain toolchain)
5956
{
@@ -73,7 +70,7 @@ public void MemoryDiagnoserIsAccurate(IToolchain toolchain)
7370
});
7471
}
7572

76-
[FactEnvSpecific("We don't want to test NativeAOT twice (for .NET Framework 4.6.2 and .NET 8.0)", EnvRequirement.DotNetCoreOnly, Skip = AllocatingSkipReason)]
73+
[FactEnvSpecific("We don't want to test NativeAOT twice (for .NET Framework 4.6.2 and .NET 8.0)", EnvRequirement.DotNetCoreOnly)]
7774
public void MemoryDiagnoserSupportsNativeAOT()
7875
{
7976
if (RuntimeInformation.IsMacOS())
@@ -82,7 +79,7 @@ public void MemoryDiagnoserSupportsNativeAOT()
8279
MemoryDiagnoserIsAccurate(NativeAotToolchain.Net80);
8380
}
8481

85-
[FactEnvSpecific("We don't want to test MonoVM twice (for .NET Framework 4.6.2 and .NET 8.0)", EnvRequirement.DotNetCoreOnly, Skip = AllocatingSkipReason)]
82+
[FactEnvSpecific("We don't want to test MonoVM twice (for .NET Framework 4.6.2 and .NET 8.0)", EnvRequirement.DotNetCoreOnly)]
8683
public void MemoryDiagnoserSupportsModernMono()
8784
{
8885
MemoryDiagnoserIsAccurate(MonoToolchain.Mono80);
@@ -156,7 +153,7 @@ public void TieredJitShouldNotInterfereAllocationResults(IToolchain toolchain)
156153
{
157154
{ nameof(NoAllocationsAtAll.TimeConsuming), 0 }
158155
},
159-
iterationCount: 10); // 1 iteration is not enough to repro the problem
156+
disableTieredJit: false, iterationCount: 10); // 1 iteration is not enough to repro the problem
160157
}
161158

162159
public class NoBoxing
@@ -210,7 +207,7 @@ public void WithOperationsPerInvoke()
210207
private void DoNotInline(object left, object right) { }
211208
}
212209

213-
[Theory(Skip = AllocatingSkipReason), MemberData(nameof(GetToolchains))]
210+
[Theory, MemberData(nameof(GetToolchains))]
214211
[Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
215212
public void AllocatedMemoryShouldBeScaledForOperationsPerInvoke(IToolchain toolchain)
216213
{
@@ -236,7 +233,7 @@ public byte[] SixtyFourBytesArray()
236233
}
237234
}
238235

239-
[TheoryEnvSpecific("Full Framework cannot measure precisely enough for low invocation counts.", EnvRequirement.DotNetCoreOnly), MemberData(nameof(GetToolchains), Skip = AllocatingSkipReason)]
236+
[TheoryEnvSpecific("Full Framework cannot measure precisely enough for low invocation counts.", EnvRequirement.DotNetCoreOnly), MemberData(nameof(GetToolchains))]
240237
[Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
241238
public void AllocationQuantumIsNotAnIssueForNetCore21Plus(IToolchain toolchain)
242239
{
@@ -301,7 +298,7 @@ public void Allocate()
301298
}
302299
}
303300

304-
[TheoryEnvSpecific("Full Framework cannot measure precisely enough", EnvRequirement.DotNetCoreOnly, Skip = AllocatingSkipReason)]
301+
[TheoryEnvSpecific("Full Framework cannot measure precisely enough", EnvRequirement.DotNetCoreOnly)]
305302
[MemberData(nameof(GetToolchains))]
306303
[Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
307304
public void MemoryDiagnoserIsAccurateForMultiThreadedBenchmarks(IToolchain toolchain)
@@ -316,9 +313,9 @@ public void MemoryDiagnoserIsAccurateForMultiThreadedBenchmarks(IToolchain toolc
316313
});
317314
}
318315

319-
private void AssertAllocations(IToolchain toolchain, Type benchmarkType, Dictionary<string, long> benchmarksAllocationsValidators, int iterationCount = 1)
316+
private void AssertAllocations(IToolchain toolchain, Type benchmarkType, Dictionary<string, long> benchmarksAllocationsValidators, bool disableTieredJit = true, int iterationCount = 1)
320317
{
321-
var config = CreateConfig(toolchain, iterationCount);
318+
var config = CreateConfig(toolchain, disableTieredJit, iterationCount);
322319
var benchmarks = BenchmarkConverter.TypeToBenchmarks(benchmarkType, config);
323320

324321
var summary = BenchmarkRunner.Run(benchmarks);
@@ -354,19 +351,32 @@ private void AssertAllocations(IToolchain toolchain, Type benchmarkType, Diction
354351
}
355352
}
356353

357-
private IConfig CreateConfig(IToolchain toolchain, int iterationCount = 1) // Single iteration is enough for most of the tests.
358-
=> ManualConfig.CreateEmpty()
359-
.AddJob(Job.ShortRun
360-
.WithEvaluateOverhead(false) // no need to run idle for this test
361-
.WithWarmupCount(0) // don't run warmup to save some time for our CI runs
362-
.WithIterationCount(iterationCount)
363-
.WithGcForce(false)
364-
.WithGcServer(false)
365-
.WithGcConcurrent(false)
366-
.WithToolchain(toolchain))
354+
private IConfig CreateConfig(IToolchain toolchain,
355+
// Tiered JIT can allocate some memory on a background thread, let's disable it by default to make our tests less flaky (#1542).
356+
// This was mostly fixed in net7.0, but tiered jit thread is not guaranteed to not allocate, so we disable it just in case.
357+
bool disableTieredJit = true,
358+
// Single iteration is enough for most of the tests.
359+
int iterationCount = 1)
360+
{
361+
var job = Job.ShortRun
362+
.WithEvaluateOverhead(false) // no need to run idle for this test
363+
.WithWarmupCount(0) // don't run warmup to save some time for our CI runs
364+
.WithIterationCount(iterationCount)
365+
.WithGcForce(false)
366+
.WithGcServer(false)
367+
.WithGcConcurrent(false)
368+
// To prevent finalizers allocating out of our control, we hang the finalizer thread.
369+
// https://github.com/dotnet/runtime/issues/101536#issuecomment-2077647417
370+
.WithEnvironmentVariable(Engines.Engine.UnitTestBlockFinalizerEnvKey, Engines.Engine.UnitTestBlockFinalizerEnvValue)
371+
.WithToolchain(toolchain);
372+
return ManualConfig.CreateEmpty()
373+
.AddJob(disableTieredJit
374+
? job.WithEnvironmentVariable("COMPlus_TieredCompilation", "0")
375+
: job)
367376
.AddColumnProvider(DefaultColumnProviders.Instance)
368377
.AddDiagnoser(MemoryDiagnoser.Default)
369378
.AddLogger(toolchain.IsInProcess ? ConsoleLogger.Default : new OutputLogger(output)); // we can't use OutputLogger for the InProcess toolchains because it allocates memory on the same thread
379+
}
370380

371381
// note: don't copy, never use in production systems (it should work but I am not 100% sure)
372382
private int CalculateRequiredSpace<T>()

0 commit comments

Comments
 (0)