Do you ever find yourself in a loop calling a method that expects an Action or a Func as an argument? Here’s an example from an EasyNetQ test method where I’m doing just that:
[Test, Explicit("Needs a Rabbit instance on localhost to work")]
public void Should_be_able_to_do_simple_request_response_lots()
{
for (int i = 0; i < 1000; i++)
{
var request = new TestRequestMessage { Text = "Hello from the client! " + i.ToString() };
bus.Request<TestRequestMessage, TestResponseMessage>(request, response =>
Console.WriteLine("Got response: '{0}'", response.Text));
}
Thread.Sleep(1000);
}
My initial naive implementation of IBus.Request set up a new response subscription each time Request was called. Obviously this is inefficient. It would be much nicer if I could identify when Request is called more than once with the same callback and re-use the subscription.
The question I had was: how can I uniquely identify each callback? It turns out that action.Method.GetHashcode() reliably identifies a unique action. I can demonstrate this with the following code:
public class UniquelyIdentifyDelegate
{
readonly IDictionary<int, Action> actionCache = new Dictionary<int, Action>();public void DemonstrateActionCache()
{
for (var i=0; i < 3; i++)
{
RunAction(() => Console.Out.WriteLine("Hello from A {0}", i));
RunAction(() => Console.Out.WriteLine("Hello from B {0}", i));
Console.Out.WriteLine("");
}
}
public void RunAction(Action action)
{
Console.Out.WriteLine("Mehod = {0}, Cache Size = {1}", action.Method.GetHashCode(), actionCache.Count);
if (!actionCache.ContainsKey(action.Method.GetHashCode()))
{
actionCache.Add(action.Method.GetHashCode(), action);
}
var actionFromCache = actionCache[action.Method.GetHashCode()];
actionFromCache();
}
}
Here, I’m creating an action cache keyed on the action method’s hashcode. Then I’m calling RunAction a few times with two distinct action delegates. Note that they also close over a variable, i, from the outer scope.
Running DemonstrateActionCache() outputs the expected result:
Mehod = 59022676, Cache Size = 0
Hello from A 0
Mehod = 62968415, Cache Size = 1
Hello from B 0
Mehod = 59022676, Cache Size = 2
Hello from A 1
Mehod = 62968415, Cache Size = 2
Hello from B 1
Mehod = 59022676, Cache Size = 2
Hello from A 2
Mehod = 62968415, Cache Size = 2
Hello from B 2
Rather nice I think :)
Mike,
ReplyDeleteI don't see the benefit of the cache. The compiler does that very optimization for you in a for loop. If you comment out the cache logic from the RunAcion method and run it you get the same result.
If you want to avoid creating a new object per lambda outside a iteration you could use some more or less elaborate assignment scheme.
Hash codes are not unique of course. You state the opposite.
ReplyDeleteHi oskark,
ReplyDeleteSorry, you right, my example is a bad one. Of course the compiler is compiling one example of each lambda, regardless of whether they are called in a loop or not. My cache doesn't add anything as it stands. What I wanted to demonstrate is that other code inside the RunAction can use the action method's hashcode to determine if it's been called with this particular delegate before.
Hi Anonymous,
Correct, hashcodes are not unique, but they are unique enough for this :)
It seems like if you had a collision from GetHashCode, you'd end up running the first Action you had cached rather than the new one.
ReplyDeleteIs there some reason that these Actions won't ever have a hash collision?
What if you add var x = i; and use x instead of i as the argument to the anonymous method?
ReplyDeleteThinking that hashcodes are unique is a common mistake. In reality, there will be two unique MethodInfo instances with the same hashcode (but would still return false for .Equals). I don't agree with your comment to Anonymous, your code is going to behave very oddly if you ever do get a collision.
ReplyDeleteI would go as far as to say that using an object's hashcode as a dictionary key is an antipattern (especially since the Dictionary class (correctly) uses the hashcode anyway)
You can fix this by keeping a Dictionary instead.
Further reading:
http://stackoverflow.com/questions/371328/why-is-it-important-to-override-gethashcode-when-equals-method-is-overriden-in-c
http://msdn.microsoft.com/en-us/library/system.object.gethashcode.aspx
Hi Rob,
ReplyDeleteThanks for the comment and links. I agree with everything you say, but I think in this very particular instance what I'm doing is probably OK. Here's my thinking...
MethodInfo.GetHashcode is not overriden so it just uses the default Object.GetHashCode(). According to this SO question:
http://stackoverflow.com/questions/720177/default-implementation-for-object-gethashcode
"The default implementation returns an index for the object determined by the common language runtime. The index is unique to an instance of an object within an AppDomain for an instance of the executing engine."
Since the MethodInfo instance will last the lifetime of the application I would expect the hashcode to also be unique for the application's lifetime.
Mike,
ReplyDeleteI completely agree with Rob. What you are relying on currently is the implementation of getHashCode, which can only be a bad thing.
In fact if you look at the documentation for it, it clearly states "Furthermore, the .NET Framework does not guarantee the default implementation of the GetHashCode method, and the value it returns will be the same between different versions of the .NET Framework".
This is evident by people who relied on the implementation of string's getHashCode, which has changed across different versions of the CLR.
Switch to a dictionary and you will be safe.