Skip to content

Conversation

@martincostello
Copy link
Member

@martincostello martincostello commented May 30, 2020

Reuse the same model binders for CancellationTokens and services, rather than allocating a new one for every request with such a parameter.

Both CancellationTokenModelBinder and ServicesModelBinder don't have any state, so it seems that reusing the same instance within the model binder providers (created lazily) would reduce the number of allocations for an application with a controller method similar to the example below:

public async Task<IActionResult> GetForecast(
    string location,
    [FromServices] WeatherClient client,
    CancellationToken cancellationToken)
{
    var forecast = await client.GetForecastAsync(location, cancellationToken);
    return Json(forecast);
}

Reuse the same model binders for CancellationTokens and services, rather than allocating a new one for every request with such a parameter.
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 30, 2020
Make the fields backing the binder once when initialized, rather than on first use.

Co-Authored-By: Pranav K <[email protected]>
@pranavkm pranavkm added this to the 5.0.0-preview7 milestone Jun 3, 2020
Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

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

Thanks!

@ghost
Copy link

ghost commented Jun 3, 2020

Hello @pranavkm!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@pranavkm pranavkm merged commit 675bcec into dotnet:master Jun 3, 2020
@martincostello martincostello deleted the Reuse-Model-Binders branch June 25, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants