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.23k stars 9.95k forks source link

Slow model binding for an object that has Func<> properties #27709

Closed andersekdahl closed 3 years ago

andersekdahl commented 3 years ago

Describe the bug

We have a context class that describes the current context, it has properties such as the current language etc. To remove boilerplate we use model binding so controller actions can receive that as an argument instead of having to take the context factory in the constructor.

Some of the properties are Func<string> instead of string because they can have a cost of loading, so we want to defer that cost until the property is actually used.

We recently started to look at performance, and noticed that these funcs makes model binding incredibly costly, each func adds around ~40ms to the total response time. Just replacing a Func with either a string or a method that returns a string removes all of the cost.

To Reproduce

Add code that looks something like this:

public class MyController : Controller
{
    public IActionResult Index([ModelBinder(typeof(MyModelBinder))] MyModel myModel)
    {
        return Ok(myModel);
    }
}

public class MyModel
{
    public Func<string> Property1 { get; set; }
    public Func<string> Property2 { get; set; }
    public Func<string> Property3 { get; set; }
}

public class MyModelBinder : IModelBinder
{
    public Task BindModelAsync(ModelBindingContext bindingContext)
    {
        bindingContext.Result = ModelBindingResult.Success(new MyModel
        {
            Property1 = () => "string1",
            Property2 = () => "string2",
            Property3 = () => "string3",
        });
        return Task.CompletedTask;
    }
}

See that response times are not what you expect. This repro actually gave me local response times of several seconds which is far worse than our real app. Could perhaps be that I'm using [ModelBinder] in this repro which we don't otherwise.

Replace the Func with regular strings and see that response times are back to normal, a couple of ms on my machine.

Further technical details

Runtime Environment: OS Name: Windows OS Version: 10.0.17763 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.401\

Host (useful for support): Version: 3.1.7 Commit: fcfdef8d6b

.NET Core SDKs installed: 2.1.202 [C:\Program Files\dotnet\sdk] 2.1.505 [C:\Program Files\dotnet\sdk] 2.1.508 [C:\Program Files\dotnet\sdk] 2.1.517 [C:\Program Files\dotnet\sdk] 2.2.108 [C:\Program Files\dotnet\sdk] 3.0.100 [C:\Program Files\dotnet\sdk] 3.1.201 [C:\Program Files\dotnet\sdk] 3.1.401 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.21 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]



- The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version
Visual Studio 2019 16.7.2
javiercn commented 3 years ago

@andersekdahl thanks for contacting us.

This is likely something in the modelbinding pipeline (model metadata provider, model binder, validator) having problems with Func<T>.

This is definitely not something we optimized for, so I would suggest you avoid that path and instead return an object with methods on it.

We might investigate this further and do something about it if there is an easy fix, but this is not a common scenario we've received significant requests to support from people, so it's not a high priority for us.

If you need a func because you need to "vary" the way some of these things are computed, (you have multiple implementations collapsed inside a func) then I would suggest you store them as fields in your context class and have the methods delegate to them instead.

ghost commented 3 years ago

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

andersekdahl commented 3 years ago

I don't mind having to work around it, I think it's reasonable that it's not a case that has or will be optimized for. It would however be nice if the system threw an exception when this happen since it has such a big performance impact.