aspnet / AspNetKatana

Microsoft's OWIN implementation, the Katana project
Apache License 2.0
959 stars 331 forks source link

Requests are run on IOCP threads by default in OwinHttpListener #508

Closed IliaBrahinets closed 7 months ago

IliaBrahinets commented 12 months ago

Consider the following sample which is built using Microsoft.Owin.SelfHost package :

static void Main(string[] args)
{
    var address = "http://localhost:8000/";

    using (var server = WebApp.Start(address, appBuilder =>
    {
        var owinHttpListener = appBuilder.Properties[typeof(OwinHttpListener).FullName] as OwinHttpListener;

        appBuilder.Use(async (context, next) =>
        {
            // This is running on IOCP thread
            Task.Delay(100).GetAwaiter().GetResult();

            await context.Response.WriteAsync("Hello world!");
        });

    }))

    {
        Console.WriteLine("Listening on " + address);
        Console.ReadKey();
    }
}

Seems like requests are processed within IOCP threads by default. Blocking of IOCP threads can make application performance worse, especially when an expensive work is performed. Is there any way to configure it ? This behaviour could be made configurable in OwinHttpListener.

Tratcher commented 12 months ago

How can you tell it's an IOCP thread?

Yes, it would likely stay on the original thread until the first async operation, then jump to the threadpool. https://github.com/aspnet/AspNetKatana/blob/3c194663090eeea35e5ee95cbe54959e0b90e3e3/src/Microsoft.Owin.Host.HttpListener/OwinHttpListener.cs#L215-L252

Middleware could pretty easily dispatch off to the threadpool. await Task.Yeild() might be enough.

FYI this repo is not under active development, it's only receiving security fixes and community contributions. We recommend moving to AspNetCore.

IliaBrahinets commented 12 months ago

Can be proved by this app:

static void Main(string[] args)
{
    ThreadPool.GetMaxThreads(out var maxWorkerThreads, out var maxCompletionPortThreads);

    Task.Run(async () =>
    {
        while (true)
        {

            ThreadPool.GetAvailableThreads(out var workerThreads, out var completionPortThreads);
            Console.WriteLine($"Worker: {maxWorkerThreads - workerThreads} Completion Port: {maxCompletionPortThreads - completionPortThreads}");
            await Task.Delay(1000);
        }
    });

    var address = "http://localhost:8000/";

    using (var server = WebApp.Start(address, appBuilder =>
    {
        var owinHttpListener = appBuilder.Properties[typeof(OwinHttpListener).FullName] as OwinHttpListener;

        appBuilder.Use(async (context, next) =>
        {
            // This is running on IOCP thread
            Thread.Sleep(Timeout.Infinite);

            await context.Response.WriteAsync("Hello world!");
        });

    }))

    {
        Console.WriteLine("Listening on " + address);
        Console.ReadKey();
    }
}

After ~50 requests I observed the following in logs: Worker: 14 Completion Port: 37. So, amount of IOCP threads is increased. It is interesting that sometimes worker threads are utilized, seems like it depends whether context switch is performed or not (probably when there are already queued requests/connections): https://github.com/aspnet/AspNetKatana/blob/3c194663090eeea35e5ee95cbe54959e0b90e3e3/src/Microsoft.Owin.Host.HttpListener/OwinHttpListener.cs#L215-L252 ASP.NET core has the option called UnsafePreferInlineScheduling which is false by default, so requests are scheduled on worker threads. https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.transport.sockets.sockettransportoptions.unsafepreferinlinescheduling?view=aspnetcore-7.0 I know that it can be mitigated by an additional middleware, but it is a boilerplate code and can be quite a surpise for consumers. IMO It would be better to have it configurable in a similar way to ASP.NET core. Wouldn't you mind if try to do PR for this ? If i had been able to use ASP.NET core I wouldn't have written here :) Unfortunately I need to use this for some time before complete migration.

IliaBrahinets commented 12 months ago

Also IOCP thread can be detected by call stack:

>   ConsoleApp1.exe!ConsoleApp1.Program.Main.AnonymousMethod__0_2(Microsoft.Owin.IOwinContext context, System.Func<System.Threading.Tasks.Task> next) Line 36   C#
    Microsoft.Owin.Hosting.dll!Microsoft.Owin.Hosting.Utilities.Encapsulate.Invoke(System.Collections.Generic.IDictionary<string, object> environment)  Unknown
    Microsoft.Owin.Host.HttpListener.dll!Microsoft.Owin.Host.HttpListener.OwinHttpListener.ProcessRequestAsync(System.Net.HttpListenerContext context)  Unknown
    Microsoft.Owin.Host.HttpListener.dll!Microsoft.Owin.Host.HttpListener.OwinHttpListener.ProcessRequestsAsync()   Unknown
    [Resuming Async Method] 
    mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
    mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.Run()    Unknown
    mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.OutputAsyncCausalityEvents.AnonymousMethod__0() Unknown
    mscorlib.dll!System.Runtime.CompilerServices.TaskAwaiter.OutputWaitEtwEvents.AnonymousMethod__0()   Unknown
    mscorlib.dll!System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Action action, bool allowInlining, ref System.Threading.Tasks.Task currentTask)    Unknown
    mscorlib.dll!System.Threading.Tasks.Task.FinishContinuations()  Unknown
    mscorlib.dll!System.Threading.Tasks.Task<System.__Canon>.TrySetResult(System.__Canon result)    Unknown
    mscorlib.dll!System.Threading.Tasks.TaskFactory<System.__Canon>.FromAsyncCoreLogic(System.IAsyncResult iar, System.Func<System.IAsyncResult, System.__Canon> endFunction, System.Action<System.IAsyncResult> endAction, System.Threading.Tasks.Task<System.__Canon> promise, bool requiresSynchronization)  Unknown
    mscorlib.dll!System.Threading.Tasks.TaskFactory<System.Net.HttpListenerContext>.FromAsyncImpl.AnonymousMethod__0(System.IAsyncResult iar)   Unknown
    System.dll!System.Net.LazyAsyncResult.Complete(System.IntPtr userToken) Unknown
    System.dll!System.Net.LazyAsyncResult.ProtectedInvokeCallback(object result, System.IntPtr userToken)   Unknown
    System.dll!System.Net.ListenerAsyncResult.IOCompleted(System.Net.ListenerAsyncResult asyncResult, uint errorCode, uint numBytes)    Unknown
    mscorlib.dll!System.Threading._IOCompletionCallback.PerformIOCompletionCallback(uint errorCode, uint numBytes, System.Threading.NativeOverlapped* pOVERLAP) Unknown

The last lines show it

Tratcher commented 12 months ago

Yes, you're welcome to submit a PR to add a similar option, but please don't change the default behavior.

TimLovellSmith commented 7 months ago

Apps that artificially block threads for long periods of times using synchronous APIs, are just likely to behave badly for using too many threads in any case!

Is there a more reasonable scenario that you'd expect to behave well but it doesn't, as opposed to one which would just be expected to behave badly in any case?