Skip to content

Commit

Permalink
Avoid lock contention during thread creation of thread pool (#108135)
Browse files Browse the repository at this point in the history
For a worker thread, Thread.StartCore and Thread.SetThreadPoolWorkerThreadName have lock contention.

Fix #108057
  • Loading branch information
AlanLiu90 authored Sep 23, 2024
1 parent 02e95eb commit 65061ad
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,12 @@ private static void CreateWorkerThread()
Thread workerThread = new Thread(s_workerThreadStart);
workerThread.IsThreadPoolThread = true;
workerThread.IsBackground = true;
// thread name will be set in thread proc
workerThread.SetThreadPoolWorkerThreadName();
workerThread.UnsafeStart();
}

private static void WorkerThreadStart()
{
Thread.CurrentThread.SetThreadPoolWorkerThreadName();

PortableThreadPool threadPoolInstance = ThreadPoolInstance;

if (NativeRuntimeEventSource.Log.IsEnabled())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ public string? Name

internal void SetThreadPoolWorkerThreadName()
{
Debug.Assert(this == CurrentThread);
Debug.Assert(ThreadState.HasFlag(ThreadState.Unstarted) || this == CurrentThread);
Debug.Assert(IsThreadPoolThread);

lock (this)
Expand Down
62 changes: 62 additions & 0 deletions src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,68 @@ public static unsafe void ThreadPoolCompletedWorkItemCountTest()
}).Dispose();
}

[ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))]
public static void ThreadPoolThreadCreationDoesNotCauseLockContention()
{
// Run in a separate process to test in a clean thread pool environment such that work items queued by the test
// would cause the thread pool to create threads
RemoteExecutor.Invoke(() =>
{
int processorCount = Environment.ProcessorCount;
var values = new double[processorCount];
StartBusyThreads();
long lockContentionCount = Monitor.LockContentionCount;
var done = new ManualResetEvent(false);
int count = 0;
for (int i = 0; i < processorCount; i++)
{
ThreadPool.QueueUserWorkItem(m =>
{
if (Interlocked.Increment(ref count) == processorCount)
{
done.Set();
}
});
}
done.WaitOne();
Assert.Equal(0, Monitor.LockContentionCount - lockContentionCount);
void StartBusyThreads()
{
using var sem = new Semaphore(0, values.Length);
for (int i = 0; i < values.Length; i++)
{
int index = i;
var t = new Thread(_ =>
{
sem.Release();
while (true)
{
for (int j = 0; j < int.MaxValue; j++)
{
values[index] += Math.Sqrt(j);
}
}
});
t.Start();
}
for (int i = 0; i < values.Length; i++)
{
sem.WaitOne();
}
}
}).Dispose();
}

public static bool IsThreadingAndRemoteExecutorSupported =>
PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported;

Expand Down

0 comments on commit 65061ad

Please sign in to comment.