Optimize expression compilation memory usage #429
Conversation
|
/cc @rynowak |
| private List<IDisposable> _transientDisposables; | ||
|
|
||
| internal ServiceProvider Root { get; } |
There was a problem hiding this comment.
Do we generally use internal or public on classes that are already internal?
There was a problem hiding this comment.
I don't have strong opinion on this, internal property in internal class seems to be pointless now when I look at it.
|
@sajayantony Do you still have the application mentioned in aspnet/Hosting#781 that was used in testing the impact of #426? |
|
I really like the design of this btw @pakrym 👍 |
|
|
||
| namespace Microsoft.Extensions.DependencyInjection.ServiceLookup | ||
| { | ||
| internal class CallSiteExpressionBuilder: CallSiteVisitor<ParameterExpression, Expression> |
There was a problem hiding this comment.
nit: formatting (Space after r in builder)
pakrym
force-pushed
the
pakrym/expression-tree
branch
from
d68ced6
to
9d6498b
Compare
| { | ||
| return VisitServiceScopeService(serviceScopeService, argument); | ||
| } | ||
| throw new NotSupportedException("Call site type is not supported"); |
There was a problem hiding this comment.
In the absence of proper pattern matching, let's change this to $"Call sit type '{callSite.GetType()}' is not supported" so we don't have to attach a debugger when we mess up.
|
Now I want to know if this makes things faster on coreclr. The smaller trees and avoiding compilation for singletons could be helpful even there.
|
| { | ||
| var serviceExpression = VisitCallSite(callSite, ProviderParameter); | ||
|
|
||
| List<Expression> body = new List<Expression>(); |
|
Customers app Before:After |
|
@pakrym Which tool are you using? |
|
@MaherJendoubi dotMemory |
Fixes: #425 by caching private field access into local variables.
Also optimizes amount of
Monitor.Enter/Exitandtry/catchwe do for scope services.Test case:
IdentitySample.Mvc project
Run
net451and request/multiple times to force all services to compile.Before
234 MB allocation from
Microsoft.Extensions.DependencyInjection.ServiceProvider+<>c__DisplayClass17_0.<RealizeService>b__1(), 223 MB because of CAS checks.And 2700ms total compilation time.
After
9.8 MB allocation from
Microsoft.Extensions.DependencyInjection.ServiceProvider+<>c__DisplayClass17_0.<RealizeService>b__1(), 4.3 MB because of CAS checks.And 400ms total compilation time.
After + No trees for singletons
2.7 MB allocation from
Microsoft.Extensions.DependencyInjection.ServiceProvider+<>c__DisplayClass17_0.<RealizeService>b__1(), 1.8 MB because of CAS checks.And 122ms total compilation time.
After + No trees for singletons + Short circuit singletons
https://github.com/aspnet/DependencyInjection/pull/429/files#diff-800d14c6aa47e810955d46f860d9502cR52
1.6 MB allocation from
Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteExpressionBuilder.Build(IServiceCallSite), 1.2 MB because of CAS checks.And 89ms total compilation time.
@halter73 @davidfowl