Improve reflection performance via e.g., `CreateDelegate()` about ontopic-library HOT 7 CLOSED

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()?

from ontopic-library.

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<,>.

from ontopic-library.

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.

from ontopic-library.

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.

from ontopic-library.

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.

from ontopic-library.

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.

from ontopic-library.

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.

from ontopic-library.