Skip to content

Commit 7e646be

Browse files
authored
Use AutoResetEvent as oppose to ManualResetEventSlim (#8575)
Summary Customer, mainly internal like XStore, with huge repos, using msbuild /graph /bl on powerful development and build computers, might experience 15x plus regression in evaluation time. It has been identified as performance bug in our logging event pub/sub mechanism. When ingest queue reaches its bound, at .net472 ManualResetEventSlim causes way too many thread.Yields flooding the system with thread context switches. This hypothesis has been verified by PerfMon perfcounter System.ContextSwitches. Alhougt counterintuitive, AutoResetEvent , ManualResetEvent or even SpinLocking produced better behavior and with those the issue no longer reproduce. Customer Impact In case of XStore it was about 7 minutes in build time. Regression? Yes, introduced in VS 17.4. Testing Manual validation by @rokonec and automated tests. Using local repro to verify changes has fixed it. Risk Low Note It effect only VS MSBuild.exe. In dotnet build ManualResetEventSlim works better.
1 parent f08e881 commit 7e646be

File tree

2 files changed

+11
-13
lines changed

2 files changed

+11
-13
lines changed

eng/Versions.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the MIT license. See License.txt in the project root for full license information. -->
33
<Project>
44
<PropertyGroup>
5-
<VersionPrefix>17.4.2</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
5+
<VersionPrefix>17.4.3</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
66
<AssemblyVersion>15.1.0.0</AssemblyVersion>
77
<PreReleaseVersionLabel>preview</PreReleaseVersionLabel>
88
<DotNetUseShippingVersions>true</DotNetUseShippingVersions>

src/Build/BackEnd/Components/Logging/LoggingService.cs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -244,15 +244,15 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler, IBu
244244
/// <summary>
245245
/// Event set when message is consumed from queue.
246246
/// </summary>
247-
private ManualResetEventSlim _dequeueEvent;
247+
private AutoResetEvent _dequeueEvent;
248248
/// <summary>
249249
/// Event set when queue become empty.
250250
/// </summary>
251-
private ManualResetEventSlim _emptyQueueEvent;
251+
private ManualResetEvent _emptyQueueEvent;
252252
/// <summary>
253253
/// Even set when message is added into queue.
254254
/// </summary>
255-
private ManualResetEventSlim _enqueueEvent;
255+
private AutoResetEvent _enqueueEvent;
256256

257257
/// <summary>
258258
/// CTS for stopping logging event processing.
@@ -1184,8 +1184,7 @@ internal virtual void ProcessLoggingEvent(object buildEvent)
11841184
while (_eventQueue.Count >= _queueCapacity)
11851185
{
11861186
// Block and wait for dequeue event.
1187-
_dequeueEvent.Wait();
1188-
_dequeueEvent.Reset();
1187+
_dequeueEvent.WaitOne();
11891188
}
11901189

11911190
_eventQueue.Enqueue(buildEvent);
@@ -1210,12 +1209,12 @@ public void WaitForLoggingToProcessEvents()
12101209
{
12111210
while (_eventQueue?.IsEmpty == false)
12121211
{
1213-
_emptyQueueEvent?.Wait();
1212+
_emptyQueueEvent?.WaitOne();
12141213
}
12151214
// To avoid race condition when last message has been removed from queue but
12161215
// not yet fully processed (handled by loggers), we need to make sure _emptyQueueEvent
12171216
// is set as it is guaranteed to be in set state no sooner than after event has been processed.
1218-
_emptyQueueEvent?.Wait();
1217+
_emptyQueueEvent?.WaitOne();
12191218
}
12201219

12211220
/// <summary>
@@ -1264,9 +1263,9 @@ private static int GetWarningsAsErrorOrMessageKey(BuildEventArgs buildEventArgs)
12641263
private void StartLoggingEventProcessing()
12651264
{
12661265
_eventQueue = new ConcurrentQueue<object>();
1267-
_dequeueEvent = new ManualResetEventSlim(false);
1268-
_emptyQueueEvent = new ManualResetEventSlim(false);
1269-
_enqueueEvent = new ManualResetEventSlim(false);
1266+
_dequeueEvent = new AutoResetEvent(false);
1267+
_emptyQueueEvent = new ManualResetEvent(false);
1268+
_enqueueEvent = new AutoResetEvent(false);
12701269
_loggingEventProcessingCancellation = new CancellationTokenSource();
12711270

12721271
_loggingEventProcessingThread = new Thread(LoggingEventProc);
@@ -1277,7 +1276,7 @@ private void StartLoggingEventProcessing()
12771276
void LoggingEventProc()
12781277
{
12791278
var completeAdding = _loggingEventProcessingCancellation.Token;
1280-
WaitHandle[] waitHandlesForNextEvent = { completeAdding.WaitHandle, _enqueueEvent.WaitHandle };
1279+
WaitHandle[] waitHandlesForNextEvent = { completeAdding.WaitHandle, _enqueueEvent };
12811280

12821281
do
12831282
{
@@ -1296,7 +1295,6 @@ void LoggingEventProc()
12961295
WaitHandle.WaitAny(waitHandlesForNextEvent);
12971296
}
12981297

1299-
_enqueueEvent.Reset();
13001298
_emptyQueueEvent.Reset();
13011299
}
13021300
} while (!_eventQueue.IsEmpty || !completeAdding.IsCancellationRequested);

0 commit comments

Comments
 (0)