Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove per enqueue volatile check in TheadPool #36697

Merged
merged 4 commits into from
May 19, 2020
Merged

Remove per enqueue volatile check in TheadPool #36697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6417499
Remove TheadPool initialization volatile
benaadams May 19, 2020
e500893
Better ThreadPoolGlobals setup for Mono
benaadams May 19, 2020
2e2f9e3
Feedback
benaadams May 19, 2020
310a2e8
Move back to lambda
benaadams May 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 7 additions & 20 deletions src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ public static partial class ThreadPool
// Time in ms for which ThreadPoolWorkQueue.Dispatch keeps executing work items before returning to the OS
private const uint DispatchQuantum = 30;

private static bool GetEnableWorkerTracking()
{
bool enableWorkerTracking = false;
InitializeVMTp(ref enableWorkerTracking);
return enableWorkerTracking;
}

internal static bool KeepDispatching(int startTickCount)
{
// Note: this function may incorrectly return false due to TickCount overflow
Expand Down Expand Up @@ -302,25 +309,6 @@ bool compressStack
public static unsafe bool UnsafeQueueNativeOverlapped(NativeOverlapped* overlapped) =>
PostQueuedCompletionStatus(overlapped);

// The thread pool maintains a per-appdomain managed work queue.
// New thread pool entries are added in the managed queue.
// The VM is responsible for the actual growing/shrinking of
// threads.
private static void EnsureInitialized()
{
if (!ThreadPoolGlobals.threadPoolInitialized)
{
EnsureVMInitializedCore(); // separate out to help with inlining
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void EnsureVMInitializedCore()
{
InitializeVMTp(ref ThreadPoolGlobals.enableWorkerTracking);
ThreadPoolGlobals.threadPoolInitialized = true;
}

// Native methods:

[MethodImpl(MethodImplOptions.InternalCall)]
Expand All @@ -346,7 +334,6 @@ private static void EnsureVMInitializedCore()

internal static void NotifyWorkItemProgress()
{
EnsureInitialized();
NotifyWorkItemProgressNative();
}

Expand Down
4 changes: 2 additions & 2 deletions ...ystem.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox b
}
else if (obj != null)
{
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ThreadPoolGlobals.s_invokeAsyncStateMachineBox, box, _value._token,
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ThreadPool.s_invokeAsyncStateMachineBox, box, _value._token,
_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
}
else
Expand Down Expand Up @@ -210,7 +210,7 @@ void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox b
}
else if (obj != null)
{
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ThreadPoolGlobals.s_invokeAsyncStateMachineBox, box, _value._token,
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ThreadPool.s_invokeAsyncStateMachineBox, box, _value._token,
_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
}
else
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox b
}
else if (obj != null)
{
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ThreadPoolGlobals.s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ThreadPool.s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
}
else
{
Expand Down Expand Up @@ -177,7 +177,7 @@ void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox b
}
else if (obj != null)
{
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ThreadPoolGlobals.s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ThreadPool.s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
}
else
{
Expand Down
71 changes: 27 additions & 44 deletions src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,6 @@

namespace System.Threading
{
internal static class ThreadPoolGlobals
{
public static volatile bool threadPoolInitialized;
public static bool enableWorkerTracking;

public static readonly ThreadPoolWorkQueue workQueue = new ThreadPoolWorkQueue();

/// <summary>Shim used to invoke <see cref="IAsyncStateMachineBox.MoveNext"/> of the supplied <see cref="IAsyncStateMachineBox"/>.</summary>
internal static readonly Action<object?> s_invokeAsyncStateMachineBox = state =>
{
if (!(state is IAsyncStateMachineBox box))
{
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.state);
return;
}

box.MoveNext();
};
}

[StructLayout(LayoutKind.Sequential)] // enforce layout so that padding reduces false sharing
internal sealed class ThreadPoolWorkQueue
{
Expand Down Expand Up @@ -552,7 +532,7 @@ public long LocalCount
/// </returns>
internal static bool Dispatch()
{
ThreadPoolWorkQueue outerWorkQueue = ThreadPoolGlobals.workQueue;
ThreadPoolWorkQueue outerWorkQueue = ThreadPool.s_workQueue;

//
// Save the start time
Expand Down Expand Up @@ -627,7 +607,7 @@ internal static bool Dispatch()
//
// Execute the workitem outside of any finally blocks, so that it can be aborted if needed.
//
if (ThreadPoolGlobals.enableWorkerTracking)
if (ThreadPool.s_enableWorkerTracking)
{
bool reportedStatus = false;
try
Expand Down Expand Up @@ -954,6 +934,21 @@ internal static void PerformWaitOrTimerCallback(_ThreadPoolWaitOrTimerCallback h

public static partial class ThreadPool
{
internal static readonly ThreadPoolWorkQueue s_workQueue = new ThreadPoolWorkQueue();
internal static readonly bool s_enableWorkerTracking = GetEnableWorkerTracking();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>Shim used to invoke <see cref="IAsyncStateMachineBox.MoveNext"/> of the supplied <see cref="IAsyncStateMachineBox"/>.</summary>
internal static readonly Action<object?> s_invokeAsyncStateMachineBox = state =>
{
if (!(state is IAsyncStateMachineBox box))
{
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.state);
return;
}

box.MoveNext();
};

[CLSCompliant(false)]
public static RegisteredWaitHandle RegisterWaitForSingleObject(
WaitHandle waitObject,
Expand Down Expand Up @@ -1080,15 +1075,13 @@ public static bool QueueUserWorkItem(WaitCallback callBack, object? state)
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.callBack);
}

EnsureInitialized();

ExecutionContext? context = ExecutionContext.Capture();

object tpcallBack = (context == null || context.IsDefault) ?
new QueueUserWorkItemCallbackDefaultContext(callBack!, state) :
(object)new QueueUserWorkItemCallback(callBack!, state, context);

ThreadPoolGlobals.workQueue.Enqueue(tpcallBack, forceGlobal: true);
s_workQueue.Enqueue(tpcallBack, forceGlobal: true);

return true;
}
Expand All @@ -1100,15 +1093,13 @@ public static bool QueueUserWorkItem<TState>(Action<TState> callBack, TState sta
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.callBack);
}

EnsureInitialized();

ExecutionContext? context = ExecutionContext.Capture();

object tpcallBack = (context == null || context.IsDefault) ?
new QueueUserWorkItemCallbackDefaultContext<TState>(callBack!, state) :
(object)new QueueUserWorkItemCallback<TState>(callBack!, state, context);

ThreadPoolGlobals.workQueue.Enqueue(tpcallBack, forceGlobal: !preferLocal);
s_workQueue.Enqueue(tpcallBack, forceGlobal: !preferLocal);

return true;
}
Expand All @@ -1126,7 +1117,7 @@ public static bool UnsafeQueueUserWorkItem<TState>(Action<TState> callBack, TSta
//
// This occurs when user code queues its provided continuation to the ThreadPool;
// internally we call UnsafeQueueUserWorkItemInternal directly for Tasks.
if (ReferenceEquals(callBack, ThreadPoolGlobals.s_invokeAsyncStateMachineBox))
if (ReferenceEquals(callBack, ThreadPool.s_invokeAsyncStateMachineBox))
{
if (!(state is IAsyncStateMachineBox))
{
Expand All @@ -1138,9 +1129,7 @@ public static bool UnsafeQueueUserWorkItem<TState>(Action<TState> callBack, TSta
return true;
}

EnsureInitialized();

ThreadPoolGlobals.workQueue.Enqueue(
s_workQueue.Enqueue(
new QueueUserWorkItemCallbackDefaultContext<TState>(callBack!, state), forceGlobal: !preferLocal);

return true;
Expand All @@ -1153,11 +1142,9 @@ public static bool UnsafeQueueUserWorkItem(WaitCallback callBack, object? state)
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.callBack);
}

EnsureInitialized();

object tpcallBack = new QueueUserWorkItemCallbackDefaultContext(callBack!, state);

ThreadPoolGlobals.workQueue.Enqueue(tpcallBack, forceGlobal: true);
s_workQueue.Enqueue(tpcallBack, forceGlobal: true);

return true;
}
Expand All @@ -1183,25 +1170,21 @@ internal static void UnsafeQueueUserWorkItemInternal(object callBack, bool prefe
{
Debug.Assert((callBack is IThreadPoolWorkItem) ^ (callBack is Task));

EnsureInitialized();

ThreadPoolGlobals.workQueue.Enqueue(callBack, forceGlobal: !preferLocal);
s_workQueue.Enqueue(callBack, forceGlobal: !preferLocal);
}

// This method tries to take the target callback out of the current thread's queue.
internal static bool TryPopCustomWorkItem(object workItem)
{
Debug.Assert(null != workItem);
return
ThreadPoolGlobals.threadPoolInitialized && // if not initialized, so there's no way this workitem was ever queued.
ThreadPoolGlobals.workQueue.LocalFindAndPop(workItem);
return s_workQueue.LocalFindAndPop(workItem);
}

// Get all workitems. Called by TaskScheduler in its debugger hooks.
internal static IEnumerable<object> GetQueuedWorkItems()
{
// Enumerate global queue
foreach (object workItem in ThreadPoolGlobals.workQueue.workItems)
foreach (object workItem in s_workQueue.workItems)
{
yield return workItem;
}
Expand Down Expand Up @@ -1239,7 +1222,7 @@ internal static IEnumerable<object> GetLocallyQueuedWorkItems()
}
}

internal static IEnumerable<object> GetGloballyQueuedWorkItems() => ThreadPoolGlobals.workQueue.workItems;
internal static IEnumerable<object> GetGloballyQueuedWorkItems() => s_workQueue.workItems;

private static object[] ToObjectArray(IEnumerable<object> workitems)
{
Expand Down Expand Up @@ -1285,7 +1268,7 @@ public static long PendingWorkItemCount
{
get
{
ThreadPoolWorkQueue workQueue = ThreadPoolGlobals.workQueue;
ThreadPoolWorkQueue workQueue = s_workQueue;
return workQueue.LocalCount + workQueue.GlobalCount + PendingUnmanagedWorkItemCount;
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/mono/netcore/System.Private.CoreLib/src/System/Threading/ThreadPool.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ namespace System.Threading
{
public static partial class ThreadPool
{
private static void EnsureInitialized()
{
ThreadPoolGlobals.threadPoolInitialized = true;
ThreadPoolGlobals.enableWorkerTracking = false;
}
private static bool GetEnableWorkerTracking() => false;

internal static void ReportThreadStatus(bool isWorking)
{
Expand Down Expand Up @@ -46,4 +42,4 @@ public static bool BindHandle(SafeHandle osHandle)

private static long PendingUnmanagedWorkItemCount => 0;
}
}
}