Improve reflection performance via e.g., CreateDelegate()

[JeremyCaney]

Use Delegate.CreateDelegate() (example, example, example) to create a delegate and cache it as part of e.g., MemberDispatcher, if not TypeMemberInfoCollection and MemberInfoCollection<>. This should dramatically improve performance.

Ideally, this should be applied to Topic classes and model classes. For Topic classes, this needs to account for Get{Attribute}() methods as well as [AttributeSetter] and [ReferenceSetter] properties. For model classes, this needs to account for properties and constructors. These may be able to be handled by a single class, though they may be better served by specialized classes.

[JeremyCaney]

Can we create a delegate for e.g., GetSetMethod() if the class type and return type aren’t known at compile time? E.g., by using:

var setter = propertyInfo.GetSetMethod();
Delegate.CreateDelegate(typeof(Action<object, object>), setter);

Or will this fail due to type mismatches with GetSetMethod()?

[JeremyCaney]

This can be done dynamically, with some creativity. The following quick-and-dirty example is adapted from Optimize C# Reflection Up to 10 Times by Using Delegates to work with GetSetMethod():

public class MethodHelper {

  private static Action<object, object> CallInnerDelegate<TClass, TParam>(Action<TClass, TParam> action)
    => (instance, value) => action((TClass)instance, (TParam)value);

  private static readonly MethodInfo CallInnerDelegateMethod = typeof(MethodHelper).GetMethod(nameof(CallInnerDelegate), BindingFlags.NonPublic | BindingFlags.Static)!;

  public void SetValue(object target, PropertyInfo propertyInfo, object value) {

    var delegateType = typeof(Action<,>).MakeGenericType(propertyInfo.DeclaringType, propertyInfo.PropertyType);
    var delegateSetter = propertyInfo.GetSetMethod().CreateDelegate(delegateType);
    var setterWithTypes = CallInnerDelegateMethod.MakeGenericMethod(propertyInfo.DeclaringType!, propertyInfo.PropertyType);
    var setter = (Action<object, object>)setterWithTypes.Invoke(null, new[] { delegateSetter })!;

    setter.Invoke(target, value);

  }
}

The delegate will be different for a get method, obviously, which will use a Func<,> instead of an Action<,>.

[JeremyCaney]

Currently, (nearly?) all calls to TypeMemberInfoCollection and MemberInfoCollection<> are routed through MemberDispatcher, which acts as a type of façade for handling the underlying complexity. Given this, we can easily swap out TypeMemberInfoCollection and MemberInfoCollection<> with e.g. a TypeAccessorCache or TypeAccessor and MemberAccessor objects without actually updating, for instance, the TrackedRecordCollection<> or TopicMappingService. That's the easiest path from a performance perspective.

That said, a number of members of the TypeMemberInfoCollection aren't really necessary anymore, or could be simplified by deeper integration with TypeAccessor and MemberAccessor. For example, the TypeMemberInfoCollection differentiates between properties and methods, whereas the MemberAccessor simply interacts with them as members; thus we could potentially consolidate e.g. SetPropertyValue() and SetMethodValue() into a single SetValue() method. Similarly, the two pieces of validation that TypeMemberInfoCollection introduces—for checking if a property has a specific attribute, and determining if value type conversion is supported—could optionally be moved to a lower level, thus preventing the need for e.g. HasSettableProperty().

Despite this, as these are internal libraries not used directly by external callers, the interface interests are a secondary priority to performance, and the MemberDispatcher façade refactoring or removal can be done independent of deploying the newly proposed TypeAccessorCache. As such, we can defer that to a separate issue if it requires further consideration.

[JeremyCaney]

Once this is fully integrated, we will be able to get rid of:

• TypeMemberInfoCollection
• MemberInfoCollection
• MemberInfoCollection<>

As well as any related unit tests.

Additionally, we should be able to effectively merge the relevant methods from MemberDispatcher into TypeAccessorCache—or, better yet, into a set of TypeAccessorCache extension methods located in the OnTopic.Mapping namespace. That’s useful since the mapping library is the only location where we need to validate and convert against the AttributeValueConverter, and it would thus keep the mapping-specific logic independent from the general reflection capabilities.

Note: Acknowledging that some mapping limits remain implicit in the MemberAccessor design, such as parameter constraints on methods.

As part of this, we can optionally collapse the following:

• HasGettableProperty(), HasGettableMethod()
• GetPropertyValue(), GetMethodValue()
• HasSettableProperty(), HasSettableMethod()
• SetPropertyValue(), SetMethodValue()

Into:

• HasGetter()
• HasSetter()
• GetValue()
• SetValue()

It may remain useful to test these independently, however.

[JeremyCaney]

One potential issue: TypeAccessor doesn’t directly contain MemberInfo objects, but rather MemberAccessor objects, which are more expensive to construct due to the compiled member accessor. This is more efficient when using reflection to read or write values, but isn’t necessary for e.g. BindingModelValidator which just requires metadata.

One alternative is to maintain a separate MemberInfoCollection for light weight metadata retrieval. And, if it‘s important, we can also add a AsTypeAccessor() method for translating to a TypeAccessor. Otherwise, the MemberInfoCollection becomes a lightweight wrapper for reflection, with no integration with MemberInfoAccessor or TypeAccessor.

[JeremyCaney]

MemberAccessor tests should focus on evaluating IsValid, properties, and data type exceptions. Standard SetValue() and GetValue() tests should go through TypeAccessorCacheExtentions, to also evaluate its validation and conversion capabilities. This can largely be based on the existing MemberDispatchTests, just as TypeAccessorCacheExtentions should be based on MemberDispatcher.

[JeremyCaney]

Not only was the original optimization made, but all of the proposed refactoring from the comments has also been introduced. This was merged into develop as part of a82f7e6. This will be released with OnTopic 5.2.0.