Skip to content

Remove ThreadPool native implementation #71719

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

Merged
merged 22 commits into from
Aug 21, 2022
Merged

Remove ThreadPool native implementation #71719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d064de8
Remove UsePortableThreadPool() and UsePortableThreadPoolForIO()
Jul 6, 2022
be8de40
Delete more unnecessary functions
Jul 7, 2022
14e1757
Delete win32threadpool.cpp and win32threadpool.h
Jul 8, 2022
a5e1c9e
Add win32threadpool with other changes
Jul 8, 2022
a2b65d1
Add win32threadpool header back
Jul 8, 2022
aa50140
Current changes go until 381/577 during build native
Jul 8, 2022
568477e
Delete GetHillClimbingLogEntry in request.cpp, Delete hillclimbing st…
Jul 9, 2022
be9e5d4
Delete hillclimbing stuff in function GetThreadpoolData in request.cpp
Jul 9, 2022
8064d92
Delete GetHillClimbingLogEntry in dacimpl.h, sospriv.idl, sospriv.h
Jul 9, 2022
9cf256c
Delete DacpHillClimbingLogEntry in dacprivate.h
Jul 9, 2022
d24f554
Revert "Delete DacpHillClimbingLogEntry in dacprivate.h"
Jul 9, 2022
c3a8389
Revert "Delete GetHillClimbingLogEntry in dacimpl.h, sospriv.idl, sos…
Jul 9, 2022
566b4d7
Change GetThreadpoolData in request.cpp to return E_NOTIMPL
Jul 9, 2022
ed44b47
Keep SOSDacLeave(); in GetThreadpoolData in request.cpp
Jul 9, 2022
b6291d5
Merge branch 'main' into update/remove-threadpool-native-implementation
jkotas Jul 9, 2022
bdb28c7
Delete mode unused code
jkotas Jul 9, 2022
98fe65e
Delete TPIndex
jkotas Jul 9, 2022
43efcf4
Update src/coreclr/System.Private.CoreLib/src/System/Threading/Thread…
jkotas Jul 10, 2022
86aa5ac
Merge branch 'main' into update/remove-threadpool-native-implementation
jkotas Aug 20, 2022
afdd6c8
CR feedback
jkotas Aug 20, 2022
e0ba959
Fix crst ordering`
jkotas Aug 20, 2022
11da1f5
Cleanup
jkotas Aug 20, 2022
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
38 changes: 0 additions & 38 deletions src/coreclr/System.Private.CoreLib/src/System/Threading/Overlapped.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,41 +25,6 @@

namespace System.Threading
{
#region class _IOCompletionCallback

internal sealed unsafe partial class _IOCompletionCallback
{
// call back helper
internal static void PerformIOCompletionCallback(uint errorCode, uint numBytes, NativeOverlapped* pNativeOverlapped)
{
do
{
OverlappedData overlapped = OverlappedData.GetOverlappedFromNative(pNativeOverlapped);

if (overlapped._callback is IOCompletionCallback iocb)
{
// We got here because of UnsafePack (or) Pack with EC flow suppressed
iocb(errorCode, numBytes, pNativeOverlapped);
}
else
{
// We got here because of Pack
var helper = (_IOCompletionCallback?)overlapped._callback;
Debug.Assert(helper != null, "Should only be receiving a completion callback if a delegate was provided.");
helper._errorCode = errorCode;
helper._numBytes = numBytes;
helper._pNativeOverlapped = pNativeOverlapped;
ExecutionContext.RunInternal(helper._executionContext, IOCompletionCallback_Context_Delegate, helper);
}

// Quickly check the VM again, to see if a packet has arrived.
OverlappedData.CheckVMForIOPacket(out pNativeOverlapped, out errorCode, out numBytes);
} while (pNativeOverlapped != null);
}
}

#endregion class _IOCompletionCallback

#region class OverlappedData

internal sealed unsafe class OverlappedData
Expand Down Expand Up @@ -120,9 +85,6 @@ internal sealed unsafe class OverlappedData

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern OverlappedData GetOverlappedFromNative(NativeOverlapped* nativeOverlappedPtr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about porting the rest of the Overlapped fcalls to managed code? Especially GetOverlappedFromNative whose implementation is trivial (just casting a pointer and a couple of dereferences) would be benefitted from being inlined by the JIT.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there are more follow up improvements that can be done here. Not as part of this PR through.


[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern void CheckVMForIOPacket(out NativeOverlapped* pNativeOverlapped, out uint errorCode, out uint numBytes);
}

#endregion class OverlappedData
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,13 +356,6 @@ internal void ResetThreadPoolThread()
Debug.Assert(this == CurrentThread);
Debug.Assert(IsThreadPoolThread);

if (!ThreadPool.UsePortableThreadPool)
{
// Currently implemented in unmanaged method Thread::InternalReset and
// called internally from the ThreadPool in NotifyWorkItemComplete.
return;
}

if (_mayNeedResetForThreadPool)
{
ResetThreadPoolThreadSlow();
Expand Down
37 changes: 8 additions & 29 deletions src/coreclr/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,19 @@ public static unsafe bool UnsafeQueueNativeOverlapped(NativeOverlapped* overlapp
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.overlapped);
}

if (UsePortableThreadPoolForIO)
{
// OS doesn't signal handle, so do it here
overlapped->InternalLow = IntPtr.Zero;

PortableThreadPool.ThreadPoolInstance.QueueNativeOverlapped(overlapped);
return true;
}
// OS doesn't signal handle, so do it here
overlapped->InternalLow = IntPtr.Zero;

return PostQueuedCompletionStatus(overlapped);
PortableThreadPool.ThreadPoolInstance.QueueNativeOverlapped(overlapped);
return true;
}

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern unsafe bool PostQueuedCompletionStatus(NativeOverlapped* overlapped);

[Obsolete("ThreadPool.BindHandle(IntPtr) has been deprecated. Use ThreadPool.BindHandle(SafeHandle) instead.")]
[SupportedOSPlatform("windows")]
public static bool BindHandle(IntPtr osHandle)
{
if (UsePortableThreadPoolForIO)
{
PortableThreadPool.ThreadPoolInstance.RegisterForIOCompletionNotifications(osHandle);
return true;
}

return BindIOCompletionCallbackNative(osHandle);
PortableThreadPool.ThreadPoolInstance.RegisterForIOCompletionNotifications(osHandle);
return true;
}

[SupportedOSPlatform("windows")]
Expand All @@ -56,22 +43,14 @@ public static bool BindHandle(SafeHandle osHandle)
{
osHandle.DangerousAddRef(ref mustReleaseSafeHandle);

if (UsePortableThreadPoolForIO)
{
PortableThreadPool.ThreadPoolInstance.RegisterForIOCompletionNotifications(osHandle.DangerousGetHandle());
return true;
}

return BindIOCompletionCallbackNative(osHandle.DangerousGetHandle());
PortableThreadPool.ThreadPoolInstance.RegisterForIOCompletionNotifications(osHandle.DangerousGetHandle());
return true;
}
finally
{
if (mustReleaseSafeHandle)
osHandle.DangerousRelease();
}
}

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern bool BindIOCompletionCallbackNative(IntPtr fileHandle);
}
}
Loading