Skip to content

Commit bb19ea4

Browse files
authored
Make counting of IO completion work items more precise on Windows (#112793)
* Make counting of IO completion work items more precise on Windows - Follow-up to #106854. Issue: #104284. - Before the change, the modified test case often yields 5 or 6 completed work items, due to queue-processing work items that happen to not process any user work items. After the change, it always yields 4. - Looks like it doesn't hurt to have more-precise counting, and there was a request to backport a fix to .NET 8, where it's more necessary to fix the issue
1 parent 38f7ca1 commit bb19ea4

File tree

3 files changed

+57
-14
lines changed

3 files changed

+57
-14
lines changed

src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs

+33-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ internal sealed class ThreadInt64PersistentCounter
1515
private static List<ThreadLocalNodeFinalizationHelper>? t_nodeFinalizationHelpers;
1616

1717
private long _overflowCount;
18+
private long _lastReturnedCount;
1819

1920
// dummy node serving as a start and end of the ring list
2021
private readonly ThreadLocalNode _nodes;
@@ -31,6 +32,13 @@ public static void Increment(object threadLocalCountObject)
3132
Unsafe.As<ThreadLocalNode>(threadLocalCountObject).Increment();
3233
}
3334

35+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
36+
public static void Decrement(object threadLocalCountObject)
37+
{
38+
Debug.Assert(threadLocalCountObject is ThreadLocalNode);
39+
Unsafe.As<ThreadLocalNode>(threadLocalCountObject).Decrement();
40+
}
41+
3442
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3543
public static void Add(object threadLocalCountObject, uint count)
3644
{
@@ -76,6 +84,17 @@ public long Count
7684
count += node.Count;
7785
node = node._next;
7886
}
87+
88+
// Ensure that the returned value is monotonically increasing
89+
long lastReturnedCount = _lastReturnedCount;
90+
if (count > lastReturnedCount)
91+
{
92+
_lastReturnedCount = count;
93+
}
94+
else
95+
{
96+
count = lastReturnedCount;
97+
}
7998
}
8099
finally
81100
{
@@ -134,6 +153,18 @@ public void Increment()
134153
OnAddOverflow(1);
135154
}
136155

156+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
157+
public void Decrement()
158+
{
159+
if (_count != 0)
160+
{
161+
_count--;
162+
return;
163+
}
164+
165+
OnAddOverflow(-1);
166+
}
167+
137168
public void Add(uint count)
138169
{
139170
Debug.Assert(count != 0);
@@ -149,7 +180,7 @@ public void Add(uint count)
149180
}
150181

151182
[MethodImpl(MethodImplOptions.NoInlining)]
152-
private void OnAddOverflow(uint count)
183+
private void OnAddOverflow(long count)
153184
{
154185
Debug.Assert(count != 0);
155186

@@ -161,7 +192,7 @@ private void OnAddOverflow(uint count)
161192
counter._lock.Acquire();
162193
try
163194
{
164-
counter._overflowCount += (long)_count + count;
195+
counter._overflowCount += _count + count;
165196
_count = 0;
166197
}
167198
finally

src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1384,6 +1384,9 @@ void IThreadPoolWorkItem.Execute()
13841384
Debug.Assert(stageBeforeUpdate != QueueProcessingStage.NotScheduled);
13851385
if (stageBeforeUpdate == QueueProcessingStage.Determining)
13861386
{
1387+
// Discount a work item here to avoid counting this queue processing work item
1388+
ThreadInt64PersistentCounter.Decrement(
1389+
ThreadPoolWorkQueueThreadLocals.threadLocals!.threadLocalCompletionCountObject!);
13871390
return;
13881391
}
13891392
}
@@ -1423,7 +1426,7 @@ void IThreadPoolWorkItem.Execute()
14231426
currentThread.ResetThreadPoolThread();
14241427
}
14251428

1426-
// Discount a work item here to avoid counting most of the queue processing work items
1429+
// Discount a work item here to avoid counting this queue processing work item
14271430
if (completedCount > 1)
14281431
{
14291432
ThreadInt64PersistentCounter.Add(tl.threadLocalCompletionCountObject!, completedCount - 1);

src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs

+20-11
Original file line numberDiff line numberDiff line change
@@ -1470,22 +1470,31 @@ public static unsafe void ThreadPoolCompletedWorkItemCountTest()
14701470
// Run in a separate process to test in a clean thread pool environment such that we don't count external work items
14711471
RemoteExecutor.Invoke(() =>
14721472
{
1473-
using var manualResetEvent = new ManualResetEventSlim(false);
1473+
const int WorkItemCount = 4;
14741474

1475-
var overlapped = new Overlapped();
1476-
NativeOverlapped* nativeOverlapped = overlapped.Pack((errorCode, numBytes, innerNativeOverlapped) =>
1475+
int completedWorkItemCount = 0;
1476+
using var allWorkItemsCompleted = new AutoResetEvent(false);
1477+
1478+
IOCompletionCallback callback =
1479+
(errorCode, numBytes, innerNativeOverlapped) =>
1480+
{
1481+
Overlapped.Free(innerNativeOverlapped);
1482+
if (Interlocked.Increment(ref completedWorkItemCount) == WorkItemCount)
1483+
{
1484+
allWorkItemsCompleted.Set();
1485+
}
1486+
};
1487+
for (int i = 0; i < WorkItemCount; i++)
14771488
{
1478-
Overlapped.Free(innerNativeOverlapped);
1479-
manualResetEvent.Set();
1480-
}, null);
1489+
ThreadPool.UnsafeQueueNativeOverlapped(new Overlapped().Pack(callback, null));
1490+
}
14811491

1482-
ThreadPool.UnsafeQueueNativeOverlapped(nativeOverlapped);
1483-
manualResetEvent.Wait();
1492+
allWorkItemsCompleted.CheckedWait();
14841493

1485-
// Allow work item(s) to be marked as completed during this time, should be only one
1486-
ThreadTestHelpers.WaitForCondition(() => ThreadPool.CompletedWorkItemCount == 1);
1494+
// Allow work items to be marked as completed during this time
1495+
ThreadTestHelpers.WaitForCondition(() => ThreadPool.CompletedWorkItemCount >= WorkItemCount);
14871496
Thread.Sleep(50);
1488-
Assert.Equal(1, ThreadPool.CompletedWorkItemCount);
1497+
Assert.Equal(WorkItemCount, ThreadPool.CompletedWorkItemCount);
14891498
}).Dispose();
14901499
}
14911500

0 commit comments

Comments
 (0)