autofac / Autofac

An addictive .NET IoC container
https://autofac.org
MIT License
4.5k stars 837 forks source link

SingleInstance lock #635

Closed nillis closed 9 years ago

nillis commented 9 years ago

We were encountering problems on our highload multithreaded application with complex dependency graph caused by the LifetimeScope lock.

Problem: The lock is provided to work around the non-threadsafe Dictionary. Although this lock is cheap, it's acquired for each SingleInstance resolve operation and shared across threads. In our case, this caused problems (high-cpu) when GC occurs and the triggering thread was blocking all other threads.

Solution: For 4.0: replace the Dictionary with a ConcurrentDictionary and remove the Monitor lock For 3.5: replace the Monitor lock with a ReaderWriterLockSlim lock (https://gist.github.com/nillis/0c63930229ac9164ff09)

Are there other known cases where this problem arises? Are there any pitfalls on the proposed solution?

tillig commented 9 years ago

We've had a couple of reports of this sort of thing in the past but we've had some challenges in addressing it:

Not that I'm saying there's no opportunity to optimize, just that we want to be very careful here.

Do you have any profiler data you could share - even a screen shot from ANTS or something like that? Before we dive into solving the problem, we just want to make sure that there's something backing up the issue. It'll also be good to ensure that once we issue a fix, we can see the hot spot go away to know we've fixed it.

Note that we're neck deep in ASP.NET v5 (vNext?) and the whole "DNX" thing right now, so we probably won't be able to do much with this until that's settled out. Lots of moving pieces. That also means it'd be solved in the 4.x codeline, not back-ported to 3.x.

nillis commented 9 years ago

The lock is only used when resolving Shared instances (f.e. SingleInstance) to ensure the thread-safety of the dictionary which contains these instances. The problem with the lock is that it's always done, also after initialization of the shared instance.

Probably the root cause of our problem isn't Autofac. But when we analyse mem dumps taken at a moment of 100% CPU peak, we always notice that the thread who's triggering GC is always stuck in the lock and blocking all other threads.

We hope with replacing the lock with the ReaderWriterLockSlim (as some components are still on .NET 3.5), we'll be able to locate the root cause of our problem which leads to a circumstance we're Autofac doesn't behave optimal.

Part of the .NET Call Stack of the GC triggering thread

Function

[[GCFrame]] 
[[HelperMethodFrame]] 
System_ni!System.Collections.Generic.Stack`1[[System.__Canon, mscorlib]].System.Collections.Generic.IEnumerable.GetEnumerator()+8d 
System_Core_ni!System.Linq.Enumerable.Any[[System.__Canon, mscorlib]](System.Collections.Generic.IEnumerable`1, System.Func`2)+5b 
Autofac.Core.Resolving.CircularDependencyDetector.CheckForCircularDependency(Autofac.Core.IComponentRegistration, System.Collections.Generic.Stack`1, Int32)+76 
Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(Autofac.Core.ISharingLifetimeScope, Autofac.Core.IComponentRegistration, System.Collections.Generic.IEnumerable`1)+5b 
Autofac.Core.Activators.Reflection.ConstructorParameterBinding.Instantiate()+9c 
Autofac.Core.Activators.Reflection.ReflectionActivator.ActivateInstance(Autofac.IComponentContext, System.Collections.Generic.IEnumerable`1)+109 
Autofac.Core.Resolving.InstanceLookup.Activate(System.Collections.Generic.IEnumerable`1)+59 
Autofac.Core.Resolving.InstanceLookup.Execute()+b5 
Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(Autofac.Core.ISharingLifetimeScope, Autofac.Core.IComponentRegistration, System.Collections.Generic.IEnumerable`1)+c8 
Autofac.Core.Activators.Reflection.ConstructorParameterBinding.Instantiate()+9c 
Autofac.Core.Activators.Reflection.ReflectionActivator.ActivateInstance(Autofac.IComponentContext, System.Collections.Generic.IEnumerable`1)+109 
Autofac.Core.Resolving.InstanceLookup.Activate(System.Collections.Generic.IEnumerable`1)+59 
Autofac.Core.Resolving.InstanceLookup.Execute()+b5 
Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(Autofac.Core.ISharingLifetimeScope, Autofac.Core.IComponentRegistration, System.Collections.Generic.IEnumerable`1)+c8 
Autofac.Core.Resolving.ResolveOperation.Execute(Autofac.Core.IComponentRegistration, System.Collections.Generic.IEnumerable`1)+39 
Autofac.ResolutionExtensions.ResolveOptionalService(Autofac.IComponentContext, Autofac.Core.Service, System.Collections.Generic.IEnumerable`1)+a8 
System.Web.Mvc.DefaultControllerFactory+DefaultControllerActivator.Create(System.Web.Routing.RequestContext, System.Type)+4e 
System.Web.Mvc.DefaultControllerFactory.CreateController(System.Web.Routing.RequestContext, System.String)+51 
System.Web.Mvc.MvcHandler.ProcessRequestInit(System.Web.HttpContextBase, System.Web.Mvc.IController ByRef, System.Web.Mvc.IControllerFactory ByRef)+125 
alexmg commented 9 years ago

The code originally had a ConcurrentDictionary but it had to be removed when we migrated to PCL because it was not available in the chosen profile. We are currently updating the dev branch to work DNX and the good news is that we can now bring it back for the DNX451 and DNXCORE50 targets.