Fix #1360: Unify the "no constructors found" reporting #1362
Conversation
Let the activator handle throwing the exception when there are no available constructors found. Enables consistent handling.
This moves the generation of the standard "no constructors found" message to the exception rather than keeping it in the activator.
Minor fix based on analyzer recommendation.
The caller can test whether the service is the right type, no need to have overloads for that. Simplifies the binder class.
After running some benchmarks, the cost to re-enumerate the in-memory collections is low compared to the cost of allocating a new list to avoid re-enumeration.
|
I did a little benchmarking and it seemed like any CPU gain from not re-enumerating the in-memory collections of services didn't balance out with the amount of memory allocated to stop re-enumeration. On a collection of around 5000 services, re-enumeration of an in-memory list cost about 300ms, while storing the list cost about 200ms and added 50KB in memory allocation for the list. (I didn't keep the results, it wasn't that scientific, and it didn't seem worth really pushing on.) Point being, it seemed reasonable to suppress the re-enumeration warnings in these cases until we get a chance to potentially refactor the code in question. I did remove some overloads in the binder that didn't seem necessary. They were getting called from basically one spot each and the caller could do the work of the overloads to allow us to short-circuit a little of the logic in the binder. It probably doesn't actually save anything measurable, but it made the binder class a little smaller and more manageable. |
ReflectionActivatorand moves it to theNoConstructorsFoundException.IConstructorFinderparameter to the exception to indicate what wasn't able to find constructors.DefaultConstructorFinder- let the activator handle throwing.I fixed an analyzer finding in the benchmark tests where we can use a local function instead of
Action<T>.There are some analyzer warnings causing the build to fail in
OpenGenericServiceBinderright now because the list of services gets enumerated multiple times. I'm not sure yet if it'd be better to allow that to happen (spend CPU) or if we should castToArrayorToListto do the enumeration once and eat the memory allocation. The source isn't from a database, so the multiple enumeration isn't really an issue from that perspective.