Design : Managing assemblies invoked via Activator.CreateInstance(...)

I am currently looking at optimising an C# (.NET 3.5) application framework, and at the moment I am looking at portions of the code that use Activator.CreateInstance to invoke assembly methods via interfaces. As an example I have the following the following, fully functional, code:

private object InvokeAssembly(string assemblyPath, string assemblyType, string data)
{
    Assembly assembly = Assembly.LoadFrom(assemblyPath);
    Type type = assembly.GetType(assemblyType, true, true);
    IMyInterface assemblyInterface = (IMyInterface)Activator.CreateInstance(type);

    return assemblyInterface.DoSomething(data);
}

The question is, is that a good design? Especially when considering that this particular method is called 100's of times a minute, so would the following singleton-esque pattern be a better design?

Improved sample code, courtesy of JaredPar

Dictionary<string, IMyInterface> assemblyCache = new Dictionary<string, IMyInterface>();

private object InvokeAssembly(string assemblyPath, string assemblyType, string data)
{
    var cacheKey = assemblyPath + "*" + assemblyType;

    IMyInterface assemblyInterface;

    if (!this.assemblyCache.TryGetValue(cacheKey, out assemblyInterface))
    {
        Assembly assembly = Assembly.LoadFrom(assemblyPath);
        开发者_如何学CType type = assembly.GetType(assemblyType, true, true);
        assemblyInterface = (IMyInterface)Activator.CreateInstance(type);
        assemblyCache[cacheKey] = assemblyInterface;
    }

    return assemblyInterface.DoSomething(data);
}

In this second example the interface is created once and reused rather than created/collected once per request.

It seems logical that the second method is the better solution, however I am more than happy to be told that it makes no difference.

It is also worth mention that this method and only this method operates on the Assembly

---

The second solution will perform better but has a bug. It cache's the IMyInterface instance and reuses it across method calls without bothering to check that the same assembly path and type were passed into the method. Consequetive calls with different path or type values will cause the wrong IMyInterface instance to be used. The caching pattern needs to be aware of this.

One way to do this is to use a Dictionary<string, IMyInterface> to cache the types.

Dictionary<string, IMyInterface> _map = new Dictionary<string, IMyInterface>();

private object InvokeAssembly(string assemblyPath, string assemblyType, string data)
{
  var key = assemblyPath + "*" + assemblyType;
  IMyInterface data;
  if (!_map.TryGetValue(key, out data)) {
    Assembly assembly = Assembly.LoadFrom(assemblyPath);
    Type type = assembly.GetType(assemblyType, true, true);
    data = (IMyInterface)Activator.CreateInstance(type);
    _map[key] = data;
  }
  return data;
}

---

The second solution is definitely going to perform better. Assemblies are cached, so it doesn't need to re-load it from disk every time, but there is still overhead involved in the GetType call and creating the instance.

---

I think the second is inferior in general -- by caching the instance created in that fashion, you run the risk of any state maintained by the the implementation of that interface to interact with the rest of your program in a potentially unpredictable way.

It's the Activator calls that are causing you performance issues, I assume? CreateInstance isn't the fastest thing in the book, after all -- a better way to do this is to create and cache lightweight delegates to the constructors of the types in question and invoke those instead. You can do this via DynamicMethod -- poking around Google for information about "factories reflection dynamicmethod" and such will get you a lot of results, here's one of them.