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.

Sign up for GitHub

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

Jump to bottom

Infrastructure: Convert more services to be singletons #6924

Closed
AndriySvyryd opened this issue Nov 2, 2016 · 1 comment
Closed

Infrastructure: Convert more services to be singletons #6924

AndriySvyryd opened this issue Nov 2, 2016 · 1 comment
Labels
area-perf breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware type-enhancement
Milestone
2.0.0

Comments

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Nov 2, 2016 •
edited by divega
Loading

There are currently some services that could be registered as singletons, reducing the number of allocations we have to make for each DbContext instance:

  1. Services that no longer need to be scoped, such as IInternalEntityEntryFactory
  2. Services that have no mutable state, but use scoped services like ICurrentDbContext or IDbContextOptions in one of the methods and can be refactored to pass the required services in the method call itself, such as IChangeTrackerFactory (we should measure the performance/memory benefit of this in a case by case basis and understand if the breaking change and possibly more coupled or complex code is worth it)
  3. Services that have a cache and could be refactored to use DbContextServices.MemoryCache, such as IValueGeneratorSelector (already tracked by Consider using Microsoft.Extensions.Caching.Memory for all caches #5322)
  4. Services that depend on one or more of the above, such as IKeyPropagator

Doing this would allow us to cache these services and not use DI to get them. On the other hand the DI resolution can be significantly sped up by not using delegates with GetService calls in them for registration, see #4133.
@divega divega added this to the 1.2.0 milestone Nov 4, 2016
@ajcvickers
Copy link
Member

Some additional thoughts on this:

  • Consider changing IDatabaseProviderServices so that it can be implemented as a singleton. This would mean that each provider would have a singleton instance, with the one being returned in the scope being the one fore the selected provider. It could then cache singleton provider services to avoid a GetService call. The service provider could no longer be injected and instead should be passed to each method that needs to resolve a scoped service.
  • Consider not allowing a different logger per context instance. The dependency on ILogger, which is scoped because a different logger can be used for each context instance, is a big factor in forcing services to be scoped.
  • Consider not allowing the model to change for each context instance, or passing the model to services that need it rather than depending on it through injection.

ajcvickers added a commit that referenced this issue Mar 11, 2017
@ajcvickers
Allow ILogger to be a singleton service
561af41 
Issues #6924 #218

This change allows ILogger to be a singleton service and hence services that need to do logging no longer need to be scoped services. The end result turned out a bit different from discussed in #218, but follows essentially the same principles. The singleton configuration API wasn't turning out well, so I switched to extending the mechanism we already have for building new service providers as necessary. This means that, when EF is managing the internal service provider, all the options work as before, but if the app changes, for example, the warnings config, then this may require a new service provider to be built. (I added a warning if more than 20 service providers are built as a heuristic against applications doing crazy stuff where the service provider is being rebuilt all the time.)

There are a few more restrictions when the internal service provider is being controlled by the application:
* For options that are directly services (ILoggerFactory, IMemoryCache) the application must configure them directly in the internal service provider. This removes the need for any re-direction.
* For other options, they can only be configured once per service provider, as was discussed in the design meeting. At the moment this is only the logging options, but it could be used for other options if necessary going forward.

Also included here is some re-shaping of the OptionsExtensions classes to bake in their immutable nature and make it harder to accidentally mutate them. WarningsConfiguration was previously mutating, and this has been updated to not mutate. Also, WarningsConfiguration and CoreOptionsExtension are needed by providers, and hence these have now been moved to a public namespace.
ajcvickers added a commit that referenced this issue Mar 11, 2017
@ajcvickers
Allow ILogger to be a singleton service
42825df 
Issues #6924 #218

This change allows ILogger to be a singleton service and hence services that need to do logging no longer need to be scoped services. The end result turned out a bit different from discussed in #218, but follows essentially the same principles. The singleton configuration API wasn't turning out well, so I switched to extending the mechanism we already have for building new service providers as necessary. This means that, when EF is managing the internal service provider, all the options work as before, but if the app changes, for example, the warnings config, then this may require a new service provider to be built. (I added a warning if more than 20 service providers are built as a heuristic against applications doing crazy stuff where the service provider is being rebuilt all the time.)

There are a few more restrictions when the internal service provider is being controlled by the application:
* For options that are directly services (ILoggerFactory, IMemoryCache) the application must configure them directly in the internal service provider. This removes the need for any re-direction.
* For other options, they can only be configured once per service provider, as was discussed in the design meeting. At the moment this is only the logging options, but it could be used for other options if necessary going forward.

Also included here is some re-shaping of the OptionsExtensions classes to bake in their immutable nature and make it harder to accidentally mutate them. WarningsConfiguration was previously mutating, and this has been updated to not mutate. Also, WarningsConfiguration and CoreOptionsExtension are needed by providers, and hence these have now been moved to a public namespace.
@ajcvickers ajcvickers mentioned this issue Mar 11, 2017
Merged
ajcvickers added a commit that referenced this issue Mar 14, 2017
@ajcvickers
Allow ILogger to be a singleton service
ecb8c15 
Issues #6924 #218

This change allows ILogger to be a singleton service and hence services that need to do logging no longer need to be scoped services. The end result turned out a bit different from discussed in #218, but follows essentially the same principles. The singleton configuration API wasn't turning out well, so I switched to extending the mechanism we already have for building new service providers as necessary. This means that, when EF is managing the internal service provider, all the options work as before, but if the app changes, for example, the warnings config, then this may require a new service provider to be built. (I added a warning if more than 20 service providers are built as a heuristic against applications doing crazy stuff where the service provider is being rebuilt all the time.)

There are a few more restrictions when the internal service provider is being controlled by the application:
* For options that are directly services (ILoggerFactory, IMemoryCache) the application must configure them directly in the internal service provider. This removes the need for any re-direction.
* For other options, they can only be configured once per service provider, as was discussed in the design meeting. At the moment this is only the logging options, but it could be used for other options if necessary going forward.

Also included here is some re-shaping of the OptionsExtensions classes to bake in their immutable nature and make it harder to accidentally mutate them. WarningsConfiguration was previously mutating, and this has been updated to not mutate. Also, WarningsConfiguration and CoreOptionsExtension are needed by providers, and hence these have now been moved to a public namespace.
ajcvickers added a commit that referenced this issue Mar 14, 2017
@ajcvickers
Allow ILogger to be a singleton service
09b1503 
Issues #6924 #218

This change allows ILogger to be a singleton service and hence services that need to do logging no longer need to be scoped services. The end result turned out a bit different from discussed in #218, but follows essentially the same principles. The singleton configuration API wasn't turning out well, so I switched to extending the mechanism we already have for building new service providers as necessary. This means that, when EF is managing the internal service provider, all the options work as before, but if the app changes, for example, the warnings config, then this may require a new service provider to be built. (I added a warning if more than 20 service providers are built as a heuristic against applications doing crazy stuff where the service provider is being rebuilt all the time.)

There are a few more restrictions when the internal service provider is being controlled by the application:
* For options that are directly services (ILoggerFactory, IMemoryCache) the application must configure them directly in the internal service provider. This removes the need for any re-direction.
* For other options, they can only be configured once per service provider, as was discussed in the design meeting. At the moment this is only the logging options, but it could be used for other options if necessary going forward.

Also included here is some re-shaping of the OptionsExtensions classes to bake in their immutable nature and make it harder to accidentally mutate them. WarningsConfiguration was previously mutating, and this has been updated to not mutate. Also, WarningsConfiguration and CoreOptionsExtension are needed by providers, and hence these have now been moved to a public namespace.
ajcvickers added a commit that referenced this issue Apr 5, 2017
@ajcvickers
Make more services singletons
a8127eb 
Issue #6924

This makes services singletons based on:
- ILogger is now singleton
- There is only one database provider per container
@ajcvickers ajcvickers mentioned this issue Apr 5, 2017
Merged
ajcvickers added a commit that referenced this issue Apr 5, 2017
@ajcvickers
Make more services singletons
c9377c4 
Issue #6924

This makes services singletons based on:
- ILogger is now singleton
- There is only one database provider per container
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 5, 2017
@ajcvickers ajcvickers changed the title Make more services singletons Infrastructure: Make more services singletons May 9, 2017
@divega divega changed the title Infrastructure: Make more services singletons Infrastructure: Convert more services to be singletons May 10, 2017
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Oct 15, 2022
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-perf breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware type-enhancement
Projects
None yet
Milestone
2.0.0
Development

No branches or pull requests
3 participants
@divega @ajcvickers @AndriySvyryd