API Proposal: Add ThreadPoolBoundHandle.AllocateUnsafeNativeOverlapped

[davidfowl]

Background and Motivation

The Http.Sys server implementation in ASP.NET Core interacts with various native windows APIs using ThreadPoolBoundHandle. Most of these interactions don't require a capture of the execution context and in some cases it's detrimental (e.g. dotnet/aspnetcore#26128). There should be a way to avoid capturing the ExecutionContext when creating a NativeOverlapped* from a ThreadPoolBoundHandle:

Proposed API

namespace System.Threading
{
    public sealed class ThreadPoolBoundHandle
    {
+       [CLSCompliant(false)]
+       public unsafe NativeOverlapped* AllocateUnsafeNativeOverlapped(IOCompletionCallback callback, object? state, object? pinData);
    }
}

[stephentoub]

The proposed API exists:
https://docs.microsoft.com/en-us/dotnet/api/system.threading.threadpoolboundhandle.allocatenativeoverlapped?view=netcore-3.1#System_Threading_ThreadPoolBoundHandle_AllocateNativeOverlapped_System_Threading_IOCompletionCallback_System_Object_System_Object_

Did you mean to suggest UnsafeAllocateNativeOverlapped (i.e. that uses Overlapped.UnsafePack instead of Overlapped.Pack)?

[stephentoub]

Ugh, nevermind, my mind is trained to see "Unsafe" at the beginning. You included it in the middle 😄

[stephentoub]

For the httpsys case, I'm curious, though: are you unable to use public unsafe NativeOverlapped* AllocateNativeOverlapped(PreAllocatedOverlapped preAllocated)?

[davidfowl]

Maybe if we refactored some codez dotnet/aspnetcore#22022. I haven't looked deeply enough but I imagine it would be possible with the right amount of effort.

[davidfowl]

@stephentoub Took a look, it's not doable as we need the closure for per call state.

[stephentoub]

Maybe if we refactored some codez dotnet/aspnetcore#22022. I haven't looked deeply enough but I imagine it would be possible with the right amount of effort.

Ok. In general PreAllocatedOverlapped leads to much lower overheads than the other overload (and doesn't capture ExecutionContext each time you call AllocateNativeOverlapped, since it's not creating a new thing), so it might be what you want and more.

[stephentoub]

Took a look, it's not doable as we need the closure for per call state.

Hmm. Ok. That's unfortunate.

[davidfowl]

Yep, this one basically requires a per request allocation 😢

[davidfowl]

Humor me a second though, what if we wanted to make this support function pointers instead of delegates:

namespace System.Threading
{
    public sealed class ThreadPoolBoundHandle
    {
+      public unsafe NativeOverlapped* AllocateUnsafeNativeOverlapped(delegate* managed<uint, uint, NativeOverlapped*, void> callback, object? state, object? pinData);
    }
}

This would bake the calling convention into the signature which might be limiting? Today we invoke the callback from managed code but I can imagine a future where we rewrite the IO thread pool to more directly call back into managed code from native code using the unmanaged calling convention and this signature would be unfortunate in that case.

namespace System.Threading
{
    public sealed class ThreadPoolBoundHandle
    {
+      public unsafe NativeOverlapped* AllocateUnsafeNativeOverlapped(delegate* unmanaged<uint, uint, NativeOverlapped*, void> callback, object? state, object? pinData);
    }
}

We could do this or directly take an IntPtr or void* but that would be really unsafe 😄 .

namespace System.Threading
{
    public sealed class ThreadPoolBoundHandle
    {
+      public unsafe NativeOverlapped* AllocateUnsafeNativeOverlapped(void* callback, object? state, object? pinData);
    }
}

Thoughts on how we might design something like this?

cc @jkotas @jaredpar

[jkotas]

I can imagine a future where we rewrite the IO thread pool to more directly call back into managed code from native code using the unmanaged calling convention

I doubt it. Everything else being equal, we are able to managed the threadpool queues more efficiently on the managed side.

What are you trying to optimize? The 3 extra instructions per work item invocation that you get with delegates compare to function pointers?

[davidfowl]

I doubt it. Everything else being equal, we are able to managed the threadpool queues more efficiently on the managed side.

I mean the IO thread pool that uses windows IO completion ports. Today a big chunk of that logic is written in native code (plus all of the native overlapped handling today that straddles native and managed). It seems there was some cost because we have this code https://source.dot.net/#System.Private.CoreLib/src/System/Threading/Overlapped.cs,53 that tries to stay in managed code for as long as possible while executing these callbacks (or maybe I'm reading that wrong).

The question was purely hypothetical though, I don't know if it's worth it to ever have function pointers in any API but if we do at some point it seems like there could be interesting questions around calling convention and function signature.

[GrabYourPitchforks]

Re: PreAllocatedOverlapped, we should consider adding a static UnsafeCreate factory to suppress EC capture when creating instances of that type. That could serve as a natural sibling to the main API proposed in this issue.

[bartonjs]

Video

• For pattern-matching, the Unsafe should be a prefix instead of an infix.
• PreAllocatedOverlapped.UnsafeCreate makes sense to add at the same time.

namespace System.Threading
{
    partial class ThreadPoolBoundHandle
    {
       [CLSCompliant(false)]
       public unsafe NativeOverlapped* UnsafeAllocateNativeOverlapped(IOCompletionCallback callback, object? state, object? pinData);
    }
}
namespace System.Threading
{
    partial class PreAllocatedOverlapped : IDisposable
    {
        [CLSCompliant(false)]
        public static PreAllocatedOverlapped UnsafeCreate(IOCompletionCallback callback, object? state, object? pinData);
    }
}