unitycontainer / interception

Unity.Interception package
Apache License 2.0
22 stars 17 forks source link

Resolve slows down when an interceptor is set #28

Closed hypercodeplace closed 5 years ago

hypercodeplace commented 5 years ago

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.

botinko commented 5 years ago

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 5 years ago

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.

hypercodeplace commented 5 years ago

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: https://github.com/unitycontainer/interception/pull/29