Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Remove closure allocations from ServiceCollectionDescriptorExtensions#44696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@benaadams
Copy link
Member

@benaadamsbenaadams commentedNov 15, 2020•
edited
Loading

Drops the closure allocations forFunc<ServiceDescriptor, Boolean> from 754 objects to 2 for startup allocations of an ASP.NET MVC app

Before

image

After

image

Contributes to#44598

nietras and jpsullivan reacted with thumbs up emoji
@ghost
Copy link

Tagging subscribers to this area:@eerhardt,@maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details
Description:

Drops the closure allocations forFunc<ServiceDescriptor, Boolean> from 754 objects to 2 for startup allocations of an ASP.NET MVC app

Before

image

After

image

Author:benaadams
Assignees:-
Labels:

area-Extensions-DependencyInjection

Milestone:-

{
// Already added
return;
}
Copy link
Member

@stephentoubstephentoubNov 15, 2020•
edited
Loading

Choose a reason for hiding this comment

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

How many times are these methods called in a typical startup? The fact that you said it's saving ~750 closures suggests it's called to add that many services, and this is O(N^2) (for each service looking at every other service already added)?

adamsitnik and nietras reacted with thumbs up emoji
Copy link
MemberAuthor

@benaadamsbenaadamsNov 15, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Yep definitely is; can't get the call counts atm as Tracing in dotTrace is broken for me atm 😢

Copy link
Member

Choose a reason for hiding this comment

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

It'd be good to understand the actual cost. While saving 750 allocations is good, I'd expect saving 250,000 interface dispatches to be better ;-)

Copy link
Member

Choose a reason for hiding this comment

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

The service collection needs to store a dictionary and introduce a new interface to check if a service type already exists

@eerhardteerhardt merged commit7426c10 intodotnet:masterNov 18, 2020
@benaadamsbenaadams deleted the ServiceCollectionDescriptorExtensions branchNovember 18, 2020 19:57
@adamsitnikadamsitnik added the tenet-performancePerformance related issue labelDec 11, 2020
@adamsitnikadamsitnik added this to the6.0.0 milestoneDec 11, 2020
@ghostghost locked asresolvedand limited conversation to collaboratorsJan 10, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@davidfowldavidfowldavidfowl left review comments

@stephentoubstephentoubstephentoub approved these changes

@eerhardteerhardteerhardt approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@benaadams@davidfowl@stephentoub@eerhardt@adamsitnik@Dotnet-GitSync-Bot
[8]ページ先頭
©2009-2025 Movatter.jp