unitycontainer/interception

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.

ENikS commented

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.

If you think you could improve performance with this idea I would love to see a PR to verify it.

@ENikS please look at this PR: #29