Reflection optimization

In a web application I'm working on I have a class that Map objects to other (different) objects. It is implemented with something like:

public interface IMapper
{
    object Map(T item);
}

public class Mapper
{
    private static ConcurrentDictionary cache = new ConcurrentDictionary();

    public Mapper()
    {
        // dependencies are injected in the constructor
        // not shown here for simplicity
    }

    public object Map(T item)
    {
        if (cache.ContainsKey(typeof(T)))
            return (cache[typeof(T)] as Func)(item);

        var method = typeof(Mapper).GetMethod("Map", BindingFlags.Public | BindingFlags.Instance, null, new[] { typeof(T) }, null);

        var @delegate = (Func)Delegate.CreateDelegate(typeof(Func), this, method);
        cache.TryAdd(typeof(T), @delegate);

        return @delegate(item);

        // I'm trying not to use this 
        //return method.Invoke(this, new object[] { item });
    }

    public object Map(Guid item) { return new object(); }
}

In order to assess what I believe should be a performance gain (compared to the .Invoke() solution) I tried to generate ten million objects (tested with strings) and then I ran the mapper for all of them and printed out the time elapsed. The results are.

• Direct call: 1.67 s

• Reflection: 14.09 s

• Optimized Reflection: 2.62 s

which is not bad actually but I wonder if I'm testing this correctly.

Test code

var guidList = new List();
        for(int i = 0; i  mapper.Map(_)).ToList();
        sw.Stop();

(Obviously, when I tested the direct method call I changed the select lambda expression to _ => mapper.Map(_))

Am I doing it right?

On a side note: I declared the ConcurrentDictionary as ConcurrentDictionary and so I have to "cast" the delegate part like this (cache[typeof(T)] as Func)(item) when I'm using it. Is there a better way to do this?

Instead of using a ConcurrentDictionary as a cache, and having to cast, you can use an embedded generic static class. The static construction is lazy and threadsafe by design. To make it work you have to create and open delegate (an instance method with no "this") for the mapper function:

public class Mapper
{
    public Mapper()
    {
        // dependencies are injected in the constructor
        // not shown here for simplicity
    }

    public object Map(T item)
    {
        var mapper = MapperCache.Map;
        return mapper(this, item);
    }

    // example methods
    public object Map(SomeObject item) { return new object(); }

    public object Map(SomeTotallyDifferentObject item) { return new object(); }

    public object Map(Guid item) { return new object(); }

    private static class MapperCache
    {
        public static readonly Func Map;

        static MapperCache()
        {
            var mapMethod = typeof(Mapper).GetMethod("Map", BindingFlags.Public | BindingFlags.Instance, null, new[] { typeof(T) }, null);

                Map = mapMethod != null ? (Func)Delegate.CreateDelegate(typeof(Func), mapMethod)
                                        : (mapper, item) => { throw new InvalidOperationException($"{nameof(Mapper)} cannot map from {typeof(T).Name} to object"); };
        }
    }
}

The first time you reference the static property, Map, the MapperCache for the given type T will be initialized. By setting up the delegate creation in the static constructor you guarantee that it will only run once for each T. If any exceptions are thrown in the static constructor the type will fail to load and cannot be used at runtime.

The same trick is used in System.Data.DataRowExtensions (source) to cache converters.

StackExchange Code Review Q#119759, answer score: 4