[Design] Reduce DI Allocations #289
Conversation
Resolves aspnet#288
|
Hi @benaadams, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
Or should it be reinitalised each time?
| @@ -19,12 +19,12 @@ internal class ServiceProvider : IServiceProvider, IDisposable | |||
| { | |||
| private readonly object _sync = new object(); | |||
|
|
|||
| private readonly Func<Type, Func<ServiceProvider, object>> _createServiceAccessor; | |||
| private Func<Type, Func<ServiceProvider, object>> _createServiceAccessor; | |||
There was a problem hiding this comment.
In error, has no state
|
|
||
| public ServiceScope(ServiceProvider scopedProvider) | ||
| { | ||
| _scopedProvider = scopedProvider; | ||
| } | ||
| internal ServiceScope(ServiceProvider scopedProvider, ServiceScopeFactory parentFactory) |
There was a problem hiding this comment.
Why internal for this? will the public constructor ever be used with these changes?
There was a problem hiding this comment.
Suppose not, missed it was an internal class
| @@ -19,9 +21,19 @@ public IServiceProvider ServiceProvider | |||
| get { return _scopedProvider; } | |||
| } | |||
|
|
|||
| internal void Reset() | |||
There was a problem hiding this comment.
Why internal here? It would also be helpful to capture your explanation of this in comments.
There was a problem hiding this comment.
Not sure what to write... Its a method that is explicitly bonded to the pooling of ServiceScopeFactory so if the class were made public rather than internal for whatever reason the exposed api would be for it to function as a standalone class (which is why I left public constructor in); whereas the "internals" only work as a class pair.
Not hugely fussed on visibility though, can make public
|
See aspnet/HttpAbstractions#402 (comment) for details on server run improvements (61.5k rps -> 1.49M rps) |
|
⌚ - just waiting on a unit test for the scenario we discussed |
|
Have the test, just trying to think of the right name XD |
|
👍 |
Conflicts: src/Microsoft.Framework.DependencyInjection/ServiceProvider.cs
Resolves #288