Skip to content

Commit a0eb8bf

Browse files
kouveleduardo-vpEduardo Manuel Velarde Polar
authored
[9.0] Make counting of IO completion work items more precise on Windows (#112794)
* Stop counting work items from ThreadPoolTypedWorkItemQueue for ThreadPool.CompletedWorkItemCount (#106854) * Stop counting work items from ThreadPoolTypedWorkItemQueue as completed work items --------- Co-authored-by: Eduardo Manuel Velarde Polar <[email protected]> Co-authored-by: Koundinya Veluri <[email protected]> * 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 --------- Co-authored-by: Eduardo Velarde <[email protected]> Co-authored-by: Eduardo Manuel Velarde Polar <[email protected]>
1 parent 296586b commit a0eb8bf

File tree

3 files changed

+77
-3
lines changed

3 files changed

+77
-3
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

+8-1
Original file line numberDiff line numberDiff line change
@@ -1375,6 +1375,9 @@ void IThreadPoolWorkItem.Execute()
13751375
Debug.Assert(stageBeforeUpdate != QueueProcessingStage.NotScheduled);
13761376
if (stageBeforeUpdate == QueueProcessingStage.Determining)
13771377
{
1378+
// Discount a work item here to avoid counting this queue processing work item
1379+
ThreadInt64PersistentCounter.Decrement(
1380+
ThreadPoolWorkQueueThreadLocals.threadLocals!.threadLocalCompletionCountObject!);
13781381
return;
13791382
}
13801383
}
@@ -1414,7 +1417,11 @@ void IThreadPoolWorkItem.Execute()
14141417
currentThread.ResetThreadPoolThread();
14151418
}
14161419

1417-
ThreadInt64PersistentCounter.Add(tl.threadLocalCompletionCountObject!, completedCount);
1420+
// Discount a work item here to avoid counting this queue processing work item
1421+
if (completedCount > 1)
1422+
{
1423+
ThreadInt64PersistentCounter.Add(tl.threadLocalCompletionCountObject!, completedCount - 1);
1424+
}
14181425
}
14191426
}
14201427

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

+36
Original file line numberDiff line numberDiff line change
@@ -1462,6 +1462,42 @@ static async Task RunAsyncIOTest()
14621462
}, ioCompletionPortCount.ToString()).Dispose();
14631463
}
14641464

1465+
1466+
[ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))]
1467+
[PlatformSpecific(TestPlatforms.Windows)]
1468+
public static unsafe void ThreadPoolCompletedWorkItemCountTest()
1469+
{
1470+
// Run in a separate process to test in a clean thread pool environment such that we don't count external work items
1471+
RemoteExecutor.Invoke(() =>
1472+
{
1473+
const int WorkItemCount = 4;
1474+
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++)
1488+
{
1489+
ThreadPool.UnsafeQueueNativeOverlapped(new Overlapped().Pack(callback, null));
1490+
}
1491+
1492+
allWorkItemsCompleted.CheckedWait();
1493+
1494+
// Allow work items to be marked as completed during this time
1495+
ThreadTestHelpers.WaitForCondition(() => ThreadPool.CompletedWorkItemCount >= WorkItemCount);
1496+
Thread.Sleep(50);
1497+
Assert.Equal(WorkItemCount, ThreadPool.CompletedWorkItemCount);
1498+
}).Dispose();
1499+
}
1500+
14651501
public static bool IsThreadingAndRemoteExecutorSupported =>
14661502
PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported;
14671503

0 commit comments

Comments
 (0)