Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DI allocations improvements #47607

Open
sebastienros opened this issue Jan 28, 2021 · 8 comments · Fixed by #50463
Open

DI allocations improvements #47607

sebastienros opened this issue Jan 28, 2021 · 8 comments · Fixed by #50463
Assignees
Milestone

Comments

@sebastienros
Copy link
Member

In Orchard CMS which is quite allocaty, the second culprit is DI. This code path does a database request and renders the html. During that process several services are resolved, increasing the size of the scoped service dictionary.

Stack trace of allocations:

Name                                                                                                                                                                                                                                  	Inc %
 Type Entry[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.Object][]                                                                                                                                   	  7.8
+ coreclr!?                                                                                                                                                                                                                           	  7.8
|+ System.Private.CoreLib.il!System.Collections.Generic.Dictionary`2[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.__Canon].Resize(int32,bool)                                                        	  7.8
||+ System.Private.CoreLib.il!System.Collections.Generic.Dictionary`2[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.__Canon].TryInsert(!0,!1,value class System.Collections.Generic.InsertionBehavior)	  7.8
|| + System.Private.CoreLib.il!System.Collections.Generic.Dictionary`2[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.__Canon].Add(!0,!1)                                                              	  7.8
||  + microsoft.extensions.dependencyinjection.il!dynamicClass.ResolveService(pMT: 00007FF84C1DFF08,pMT: 00007FF84C1D9B20)                                                                                                            	  7.8
||   + microsoft.extensions.dependencyinjection.il!dynamicClass.ResolveService(pMT: 00007FF84C1DFF08,pMT: 00007FF84C1D9B20)                                                                                                           	  7.8
||    + microsoft.extensions.dependencyinjection.abstractions.il!ServiceProviderServiceExtensions.GetRequiredService                                                                                                                  	  4.8
||    |+ microsoft.extensions.dependencyinjection.abstractions.il!ServiceProviderServiceExtensions.GetRequiredService                                                                                                                 	  4.8
||    | + orchardcore.displaymanagement.liquid!OrchardCore.DisplayManagement.Liquid.LiquidTemplateContextExtensions+<EnterScopeAsync>d__0.MoveNext()                                                                                  	  4.5

And overall memory consumption:

Name                                                                                                                                         Exc %  ExcInc %     Inc Fold                            When
?!?                                                                                                                                           8.66,57215.5  11,8571,7788389CDDoDCDFED9ECFFFDGGHFFE89DY*
e_sqlite3!?                                                                                                                                   4.03,088 6.1   4,7011,36762526767655664565557556565569535
microsoft.extensions.dependencyinjection.il!dynamicClass.ResolveService(pMT: 00007FF84C1DFF08,pMT: 00007FF84C1D9B20)                          2.92,217 5.4   4,1341,907105o556655665546566567566564630o
orchardcore.displaymanagement!OrchardCore.DisplayManagement.Implementation.DefaultHtmlDisplay+<ExecuteAsync>d__7.MoveNext()                   2.72,09728.1  21,4691,95400N5WWXVWVVVWSOXUWWXXVWXWWXRWN9.
orchardcore.contentmanagement.display!OrchardCore.ContentManagement.Display.ContentItemDisplayCoordinator+<BuildDisplayAsync>d__9.MoveNext() 2.31,74910.0   7,6481,6760_81BCABBBABB99CBBACBABBBCCBD92_
System.Private.CoreLib.il!AsyncMethodBuilderCore.Start                                                                                        2.21,69071.9  54,9611,42801****************************3_
orchardcore.displaymanagement!OrchardCore.DisplayManagement.Views.ShapeResult+<ApplyImplementationAsync>d__17.MoveNext()                      2.01,540 5.2   4,0081,4130o40665656555446655655656666741_
orchardcore.displaymanagement.liquid!OrchardCore.DisplayManagement.Liquid.Tags.HelperStatement+<WriteToAsync>d__4.MoveNext()                  1.91,47413.0   9,9651,38200A3FGFFEEEEEDBGEEEFEEEFFGGCDA51
ntoskrnl!RtlpLookupFunctionEntryForStackWalks                                                                                                 1.91,437 1.9   1,437  242__o012222223221122222o222220____
orchardcore.displaymanagement.liquid!LiquidTagHelperActivator.Create                                                                          1.91,428 2.3   1,7831,3710o202222222222132232222233222110

This issue is just to raise some concern, and maybe there is nothing better to be done.

@sebastienros sebastienros added the tenet-performance Performance related issue label Jan 28, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 28, 2021
@ghost
Copy link

ghost commented Jan 29, 2021

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

Issue Details

In Orchard CMS which is quite allocaty, the second culprit is DI. This code path does a database request and renders the html. During that process several services are resolved, increasing the size of the scoped service dictionary.

Stack trace of allocations:

Name                                                                                                                                                                                                                                  	Inc %
 Type Entry[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.Object][]                                                                                                                                   	  7.8
+ coreclr!?                                                                                                                                                                                                                           	  7.8
|+ System.Private.CoreLib.il!System.Collections.Generic.Dictionary`2[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.__Canon].Resize(int32,bool)                                                        	  7.8
||+ System.Private.CoreLib.il!System.Collections.Generic.Dictionary`2[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.__Canon].TryInsert(!0,!1,value class System.Collections.Generic.InsertionBehavior)	  7.8
|| + System.Private.CoreLib.il!System.Collections.Generic.Dictionary`2[Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceCacheKey,System.__Canon].Add(!0,!1)                                                              	  7.8
||  + microsoft.extensions.dependencyinjection.il!dynamicClass.ResolveService(pMT: 00007FF84C1DFF08,pMT: 00007FF84C1D9B20)                                                                                                            	  7.8
||   + microsoft.extensions.dependencyinjection.il!dynamicClass.ResolveService(pMT: 00007FF84C1DFF08,pMT: 00007FF84C1D9B20)                                                                                                           	  7.8
||    + microsoft.extensions.dependencyinjection.abstractions.il!ServiceProviderServiceExtensions.GetRequiredService                                                                                                                  	  4.8
||    |+ microsoft.extensions.dependencyinjection.abstractions.il!ServiceProviderServiceExtensions.GetRequiredService                                                                                                                 	  4.8
||    | + orchardcore.displaymanagement.liquid!OrchardCore.DisplayManagement.Liquid.LiquidTemplateContextExtensions+<EnterScopeAsync>d__0.MoveNext()                                                                                  	  4.5

And overall memory consumption:

Name                                                                                                                                         Exc %  ExcInc %     Inc Fold                            When
?!?                                                                                                                                           8.66,57215.5  11,8571,7788389CDDoDCDFED9ECFFFDGGHFFE89DY*
e_sqlite3!?                                                                                                                                   4.03,088 6.1   4,7011,36762526767655664565557556565569535
microsoft.extensions.dependencyinjection.il!dynamicClass.ResolveService(pMT: 00007FF84C1DFF08,pMT: 00007FF84C1D9B20)                          2.92,217 5.4   4,1341,907105o556655665546566567566564630o
orchardcore.displaymanagement!OrchardCore.DisplayManagement.Implementation.DefaultHtmlDisplay+<ExecuteAsync>d__7.MoveNext()                   2.72,09728.1  21,4691,95400N5WWXVWVVVWSOXUWWXXVWXWWXRWN9.
orchardcore.contentmanagement.display!OrchardCore.ContentManagement.Display.ContentItemDisplayCoordinator+<BuildDisplayAsync>d__9.MoveNext() 2.31,74910.0   7,6481,6760_81BCABBBABB99CBBACBABBBCCBD92_
System.Private.CoreLib.il!AsyncMethodBuilderCore.Start                                                                                        2.21,69071.9  54,9611,42801****************************3_
orchardcore.displaymanagement!OrchardCore.DisplayManagement.Views.ShapeResult+<ApplyImplementationAsync>d__17.MoveNext()                      2.01,540 5.2   4,0081,4130o40665656555446655655656666741_
orchardcore.displaymanagement.liquid!OrchardCore.DisplayManagement.Liquid.Tags.HelperStatement+<WriteToAsync>d__4.MoveNext()                  1.91,47413.0   9,9651,38200A3FGFFEEEEEDBGEEEFEEEFFGGCDA51
ntoskrnl!RtlpLookupFunctionEntryForStackWalks                                                                                                 1.91,437 1.9   1,437  242__o012222223221122222o222220____
orchardcore.displaymanagement.liquid!LiquidTagHelperActivator.Create                                                                          1.91,428 2.3   1,7831,3710o202222222222132232222233222110

This issue is just to raise some concern, and maybe there is nothing better to be done.

Author: sebastienros
Assignees: -
Labels:

area-Extensions-DependencyInjection, tenet-performance, untriaged

Milestone: -

@davidfowl
Copy link
Member

The thing that stands out to me is the dictionary resize. We could potentially improve this by specifying a capacity like max(scoped/2, 30) something like this (I picked those numbers out of thin air)

@eerhardt
Copy link
Member

@sebastienros - are there instructions on how to run this locally to investigate?

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Jan 29, 2021
@maryamariyan maryamariyan added this to the 6.0.0 milestone Jan 29, 2021
@sebastienros
Copy link
Member Author

Here is how I do it:

I can help more if you need. I can also provide this scenario as a crank command.

@davidfowl
Copy link
Member

I was discussing this with @maryamariyan and one thing we might want to try is re-using the scoped dictionary. This makes some assumption that scopes look the same and we have no idea to identify similar scopes (say by an id) so we can try it out with a small pool (say number of size of number of cores). This won't help for long running scopes in things like Blazor server, or Orleans but will do 2 things:

  • Significantly cut down the on the allocation cost of scopes short lived scopes
  • Reduce dictionary resizes when the number of scoped and transient services is high, per scope

We might also want to keep the dictionaries to a reasonable size so we don't bloat memory (maybe calling TrimExcess if we reached some number of entries threshold).

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 31, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 2, 2021
@davidfowl davidfowl self-assigned this Apr 2, 2021
@davidfowl davidfowl reopened this Apr 16, 2021
@davidfowl
Copy link
Member

Re-opening this as I reverted the change. I've added an event to the event source so we can figure out just how heterogeneous or homogenous scopes are within the same container.

@eerhardt
Copy link
Member

Moving to 7.0 since this isn't a "must have" for 6.0.

@eerhardt eerhardt modified the milestones: 6.0.0, 7.0.0 Jul 14, 2021
@eerhardt eerhardt modified the milestones: 7.0.0, Future Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants