Resolve slows down when an interceptor is set
Closed this issue · 3 comments
Problem
The problem is resolve slows down when an interceptor is set. Our company had several production incidents when it happens. It is clearly visible when resolve in multi-threaded with high degree of parallelism. If you want we can provide benchmark tests shown difference between current implementation and the suggested one (see below).
Root Cause
Let's suppose we have the following code:
var container = new UnityContainer()
.AddNewExtension<Interception>()
.RegisterType<IFoo, Foo>();
container
.Configure<Interception>()
.SetDefaultInterceptorFor<IFoo>(new InterfaceInterceptor());
container.Resolve<IFoo>(); // Resolve #1
container.Resolve<IFoo>(); // Resolve #2
Every resolve PipelineManager
is instantiated here. Then CreatePipeline
method invokes GetBaseDefinition()
twice here and here. GetBaseDefinition
uses inside locks that leads to slowing down of whole dependency resolves.
Possible Solution
Base method definitions can be cached in static ConcurrentDictionary
, for example. So CreatePipeline
can be rewritten something like this:
private static readonly ConcurrentDictionary<HandlerPipelineKey, MethodInfo> BaseMethodDefinitions =
new ConcurrentDictionary<HandlerPipelineKey, MethodInfo>();
private HandlerPipeline CreatePipeline(MethodInfo method, IEnumerable<ICallHandler> handlers)
{
var key = HandlerPipelineKey.ForMethod(method);
if (_pipelines.TryGetValue(key, out var pipeline))
{
return pipeline;
}
var baseMethodDefinition = BaseMethodDefinitions
.GetOrAdd(key, pipelineKey => method.GetBaseDefinition());
if (baseMethodDefinition == method)
{
_pipelines[key] = pipeline = new HandlerPipeline(handlers);
return pipeline;
}
_pipelines[key] = pipeline = CreatePipeline(baseMethodDefinition, handlers);
return pipeline;
}
@ENikS If you want we can prepare a PR, so you do not spend time on it. Also, as I mentioned, we can provide benchmark tests of current solution and supposed one.
I researched this problem earlier.
Lock happens in https://source.dot.net/#System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs,530
This method calls RuntimeType.GetMethodBase
which updates some internal cache in
https://source.dot.net/#System.Private.CoreLib/src/System/RtType.cs,374 in lock.
The implementation is not very efficient. I was planning to redo it once v6 is out but it might be a while.
If you think you could improve performance with this idea I would love to see a PR to verify it.