I'm trying to improve the performance of my IoC container. We are using Unity and SimpleInjector and we have a class with this constructor:
public AuditFacade(
IIocContainer container,
Func<IAuditManager> auditManagerFactory,
Func<ValidatorFactory> validatorCreatorFactory,
IUserContext userContext,
Func<ITenantManager> tenantManagerFactory,
Func<IMonitoringComponent> monitoringComponentFactory)
: base(container, auditManagerFactory, GlobalContext.CurrentTenant,
validatorCreatorFactory, userContext, tenantManagerFactory)
{
_monitoringComponent = new Lazy<IMonitoringComponent>(monitoringComponentFactory);
}
I also have another class with this constructor:
public AuditTenantComponent(Func<IAuditTenantRepository> auditTenantRepository)
{
_auditTenantRepository = new Lazy<IAuditTenantRepository>(auditTenantRepository);
}
I'm seeing that the second one gets resolved in 1 millisecond, most of the time, whereas the first one takes on average 50-60 milliseconds. I'm sure the reasoning for the slower one is because of the parameters, it has more parameters. But how can I improve the performance of this slower one? Is it the fact that we are using Func<T> as parameters? What can I change if it is causing the slowness?
There is possibly a lot to improve on your current design. These improvements can be placed in five different categories, namely:
The general consensus is that you should prefer composition over inheritance. Inheritance is often overused and often adds more complexity compared to using composition. With inheritance the derived class is strongly coupled to the base class implementation. I often see a base class being used as practical utility class containing all sorts of helper methods for cross-cutting concerns and other behavior that some of the derived classes may need.
An often better approach is to remove the base class all together and inject a service into the implementation (the AuditFacade class in your case) that exposed just the functionality that the service needs. Or in case of cross-cutting concerns, don't inject that behavior at all, but wrap the implementation with a decorator that extends the class'es behavior with cross-cutting concerns.
In your case, I think the complication is clearly happening, since 6 out of 7 injected dependencies are not used by the implementation, but are only passed on to the base class. In other words, those 6 dependencies are implementation details of the base class, while the implementation still is forced to know about them. By abstracting (part of) that base class behind a service, you can minimize the number of dependencies that AuditFacade needs to two dependencies: the Func<IMonitoringComponent> and the new abstraction. The implementation behind that abstraction will have 6 constructor dependencies, but the AuditFacade (and other implementations) are oblivious to that.
The AuditFacade depends on an IIocContainer abstraction and this is very like an implementation of the Service Locator pattern. Service Locator should be considered an anti-pattern because:
it hides a class' dependencies, causing run-time errors instead of compile-time errors, as well as making the code more difficult to maintain because it becomes unclear when you would be introducing a breaking change.
There are always better alternatives to injecting your container or an abstraction over your container into application code. Do note that at some times you might want to inject the container into factory implementations, but at long as those are placed inside your Composition Root, there's no harm in that, since Service Locator is about roles, not mechanics.
The static GlobalContext.CurrentTenant property is an implementation of the Ambient Context anti-pattern. Mark Seemann and I write about this pattern in our book:
The problems with AMBIENT CONTEXT are related to the problems with SERVICE LOCATOR. The main problems are:
- The DEPENDENCY is hidden.
- Testing becomes more difficult.
- It becomes very hard to change the DEPENDENCY based on its context. [paragraph 5.3.3]
The use in this case is really weird IMO, because you grab the current tenant from some static property from inside your constructor to pass it on to the base class. Why doesn't the base class call that property itself?
But no one should call that static property. The use of those static properties makes your code harder to read and maintain. It makes unit testing harder and since your code base will usually be littered with calls to such static, it becomes a hidden dependency; it has the same downsides as the use of Service Locator.
A Leaky Abstraction is a Dependency Inversion Principle violation, where the abstraction violates the second part of the principle, namely:
B. Abstractions should not depend on details. Details should depend on abstractions.
Although Lazy<T> is not abstractions by itself (Lazy<T> is a concrete type), it can become leaky abstraction when used as constructor argument. For instance, if you are injecting an Lazy<IMonitoringComponent> instead of an IMonitoringComponent directly (which is what you are basically doing in your code), the new Lazy<IMonitoringComponent> dependency leaks implementation details. This Lazy<IMonitoringComponent> communicates to the consumer that the used IMonitoringComponent implementation is expensive or time consuming to create. But why should the consumer care about this?
But there are more problems with this. If at one point in time the used IUserContext implementation becomes costly to create, we must start to make sweeping changes throughout the application (a violation of the Open/Closed Principle) because all IUserContext dependencies need to be changed to Lazy<IUserContext> and all consumers of that IUserContext must be changed to use userContext.Value. instead. And you'll have to change all your unit tests as well. And what happens if you forget to change one IUserContext reference to Lazy<IUserContext> or when you accidentally depend on IUserContext when you create a new class? You have a bug in your code, because at that point the user context implementation is created right away and this will cause a performance problem (this causes a problem, because that is the reason you are using Lazy<T> in the first place).
So why are we exactly making sweeping changes to our code base and polluting it with that extra layer of indirection? There is no reason for this. The fact that a dependency is costly to create is an implementation detail. You should hide it behind an abstraction. Here's an example:
public class LazyMonitoringComponentProxy : IMonitoringComponent {
private Lazy<IMonitoringComponent> component;
public LazyMonitoringComponentProxy(Lazy<IMonitoringComponent> component) {
this.component = component;
}
void IMonitoringComponent.MonitoringMethod(string someVar) {
this.component.Value.MonitoringMethod(someVar);
}
}
In this example we've hidden the Lazy<IMonitoringComponent> behind a proxy class. This allows us to replace the original IMonitoringComponent implementation with this LazyMonitoringComponentProxy without having to make any change to the rest of the applicaiton. With Simple Injector, we can register this type as follows:
container.Register<IMonitoringComponent>(() => new LazyMonitoringComponentProxy(
new Lazy<IMonitoringComponent>(container.GetInstance<CostlyMonitoringComp>));
And just as Lazy<T> can be abused as leaky abstraction, the same holds for Func<T>, especially when you're doing this for performance reasons. When applying DI correctly, there is most of the time no need to inject factory abstractions into your code such as Func<T>.
Do note that if you are injecting Lazy<T> and Func<T> all over the place, you are complicating your code base unneeded.
But besides Lazy<T> and Func<T> being leaky abstractions, the fact that you need them a lot is an indication of a problem with your application, because Injection Constructors should be simple. If constructors take a long time to run, your constructors are doing too much. Constructor logic is often hard to test and if such constructor makes a call to the database or requests data from HttpContext, verification of your object graphs becomes much harder to the point that you might skip verification all together. Skipping verification of the object graph is a terrible thing to do, because this forces you to click through the complete application to find out whether or not your DI container is configured correctly.
I hope this gives you some ideas about improving the design of your classes.