Consider replacing SynchronizedDictionary with ConcurrentDictionary #743

Activity

1. Youssef1313 commented on May 2, 2026

   

   Youssef1313

   on May 2, 2026

   Author

   More actions

   FYI @kzu as well
2. stakx commented on May 2, 2026

   

   stakx

   on May 2, 2026

   Member

   More actions

   Hi @Youssef1313, thanks for looking into it.

   I'm open to switch to a standard FCL type, but I think it would be good to have some kind of before/after benchmark showing that ConcurrentDictionary actually does remove a significant bottleneck. Could you provide something along that line? (E.g. a small BenchmarkDotNet project that we can drop into the Castle.Core solution file and run against DynamicProxy both before and after the change?)
3. Youssef1313 commented on May 4, 2026

   

   Youssef1313

   on May 4, 2026

   Author

   More actions

   See the last comment on #744. Somehow my changes actually broke the thread-safety guarantees currently on master. I don't think I'll get into investigating this deeper, but generally speaking, I think there is some sort of threadpool starvation under parallel loads that needs to be somehow addressed.
4. stakx commented on May 4, 2026

   

   stakx

   on May 4, 2026

   Last edited by stakx

   Member

   More actions

   @Youssef1313, when it comes to DynamicProxy thread safety, this here is quite possibly the most important code location:

   Core/src/Castle.Core/DynamicProxy/Generators/BaseProxyGenerator.cs

   Lines 77 to 86 in 9631074

   	var proxyType = Scope.TypeCache.GetOrAdd(GetCacheKey(), cacheKey =>
   	{
   	notFoundInTypeCache = true;
   	Logger.DebugFormat("No cached proxy type was found for target type {0}.", targetType.FullName);
   	EnsureOptionsOverrideEqualsAndGetHashCode();
   	var name = Scope.NamingScope.GetUniqueName("Castle.Proxies." + targetType.Name + "Proxy");
   	return GenerateType(name, Scope.NamingScope.SafeSubScope());
   	});

   This TypeCache.GetOrAdd must be exclusively locking, i. e. only one thread at a time may go ahead and generate its type. Almost all of DynamicProxy's code generation is kicked off here. Most other code may then silently assume that there is no other concurrent code generation going on, because we guarantee that at the above site. This is why you see those {GetOrAdd,AddOrUpdate}WithoutTakingLock (emphasis on "without taking lock") method calls almost everywhere else.

   If I remember correctly (but I could be wrong), ConcurrentDictionary.GetOrAdd has slightly different semantics: several factory lambdas may execute concurrently (at the same time) which, if true, would break DynamicProxy.