dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.14k stars 9.92k forks source link

Negotiate reflection performance improvements #25335

Open Tratcher opened 4 years ago

Tratcher commented 4 years ago

Feedback from @mconnew:

I was looking over your reflected abstraction of the negotiate code ... and noticed a significant performance problem in the code. There is lots of code which looks similar to this:

    public bool IsCompleted
    {
        get => (bool)_isCompleted.Invoke(_instance, Array.Empty<object>());
    }

The problem with calling the Invoke method on an MethodInfo is it does a whole bunch of runtime type checking to make sure the number and type of arguments is correct. There’s also a boxing and unboxing as well as a cast type check on the return value. You can use MethodInfo.CreateDelegate to create a delegate you can straight up call. Because it only does the type checking of the delegate type to the method info when you call CreateDelegate, it’s a lot cheaper to call. The generated delegate is strongly typed so the CLR just depends on the strong typing to ensure all the parameters etc are correct.


If we get a proper API for this in 6.0 then this code can be removed (https://github.com/dotnet/runtime/issues/29270). If not we may consider making some improvements.

Note it's not clear how these costs reflect on RPS. Windows Auth is already a very in-efficient process that often involves multiple request.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

davidfowl commented 3 years ago

@mconnew is there a wcf scenario we can setup and run?

mconnew commented 3 years ago

The scenario for this with WCF is no different than any other ASP.NET Core application using Windows auth. I just know from experience that the cost of using Invoke to use a reflected property is significantly higher than using a created delegate. I have no idea how that translates to actual additional cost for authentication in a running service. I opened this issue because it's a small quick change to do without any refactoring needed and there is some performance gain to it. I have a dev working on CoreWCF who is working on using the same reflection based usage of the Nego abstraction to implement Windows authentication with Token based message security and I discovered this when I was finding references for them.