Skip to content

Commit e7b29f3

Browse files
committed
Use Monitor.Wait instead of ManualResetEventSlim.
1 parent 1797d9c commit e7b29f3

File tree

2 files changed

+27
-13
lines changed

2 files changed

+27
-13
lines changed

src/BenchmarkDotNet/Engines/Engine.cs

+25-10
Original file line numberDiff line numberDiff line change
@@ -336,26 +336,32 @@ public static bool TryGetSignal(string message, out HostSignal signal)
336336
// https://github.com/dotnet/runtime/issues/101536#issuecomment-2077647417
337337
private readonly struct FinalizerBlocker : IDisposable
338338
{
339-
private readonly ManualResetEventSlim hangEvent;
339+
private readonly object hangLock;
340340

341-
private FinalizerBlocker(ManualResetEventSlim hangEvent) => this.hangEvent = hangEvent;
341+
private FinalizerBlocker(object hangLock) => this.hangLock = hangLock;
342342

343343
private sealed class Impl
344344
{
345-
private readonly ManualResetEventSlim hangEvent = new (false);
345+
// ManualResetEvent(Slim) allocates when it is waited and yields the thread,
346+
// so we use Monitor.Wait instead which does not allocate managed memory.
347+
// This behavior is not documented, but was observed with the VS Profiler.
348+
private readonly object hangLock = new ();
346349
private readonly ManualResetEventSlim enteredFinalizerEvent = new (false);
347350

348351
~Impl()
349352
{
350-
enteredFinalizerEvent.Set();
351-
hangEvent.Wait();
353+
lock (hangLock)
354+
{
355+
enteredFinalizerEvent.Set();
356+
Monitor.Wait(hangLock);
357+
}
352358
}
353359

354360
[MethodImpl(MethodImplOptions.NoInlining)]
355-
internal static (ManualResetEventSlim hangEvent, ManualResetEventSlim enteredFinalizerEvent) CreateWeakly()
361+
internal static (object hangLock, ManualResetEventSlim enteredFinalizerEvent) CreateWeakly()
356362
{
357363
var impl = new Impl();
358-
return (impl.hangEvent, impl.enteredFinalizerEvent);
364+
return (impl.hangLock, impl.enteredFinalizerEvent);
359365
}
360366
}
361367

@@ -365,17 +371,26 @@ internal static FinalizerBlocker MaybeStart()
365371
{
366372
return default;
367373
}
368-
var (hangEvent, enteredFinalizerEvent) = Impl.CreateWeakly();
374+
var (hangLock, enteredFinalizerEvent) = Impl.CreateWeakly();
369375
do
370376
{
371377
GC.Collect();
372378
// Do NOT call GC.WaitForPendingFinalizers.
373379
}
374380
while (!enteredFinalizerEvent.IsSet);
375-
return new FinalizerBlocker(hangEvent);
381+
return new FinalizerBlocker(hangLock);
376382
}
377383

378-
public void Dispose() => hangEvent?.Set();
384+
public void Dispose()
385+
{
386+
if (hangLock is not null)
387+
{
388+
lock (hangLock)
389+
{
390+
Monitor.Pulse(hangLock);
391+
}
392+
}
393+
}
379394
}
380395
}
381396
}

tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs

+2-3
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,8 @@ public byte[] SixtyFourBytesArray()
234234
}
235235
}
236236

237-
[TheoryEnvSpecific("Full Framework cannot measure precisely enough for low invocation counts.", EnvRequirement.DotNetCoreOnly), MemberData(nameof(GetToolchains),
238-
Skip = "Some random background allocations are occurring that we haven't been able to figure out, causing this test in particular to be flaky." +
239-
" Other tests likely also suffer from it, but their high invocation counts successfully drown it out. #2562")]
237+
[TheoryEnvSpecific("Full Framework cannot measure precisely enough for low invocation counts.", EnvRequirement.DotNetCoreOnly)]
238+
[MemberData(nameof(GetToolchains))]
240239
[Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
241240
public void AllocationQuantumIsNotAnIssueForNetCore21Plus(IToolchain toolchain)
242241
{

0 commit comments

Comments
 (0)