Subclassing
-
Didn't hold me up for very long, but gave me a chuckle... This is a piece of a messaging system, where the automated clients route incoming messages to more specific handlers, based on what subclass it is...
private void ReceiveMessageAsync(object state)
{
Message m = (Message)state;if (m is CommandMessage)
OnCommandReceived(m as CommandMessage);
else if (m is ReplyMessage)
OnReplyReceived(m as ReplyMessage);
else if (m is SubscriptionMessage)
{
SubscriptionMessage sm = m as SubscriptionMessage;
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
}
}I was wondering why commands and replies were going through just fine, but subscription messages would never be routed properly.. Then I remembered what a SubscriptionMessage was...
public class SubscriptionMessage : CommandMessage
Sometimes, order matters :)
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels) -
Didn't hold me up for very long, but gave me a chuckle... This is a piece of a messaging system, where the automated clients route incoming messages to more specific handlers, based on what subclass it is...
private void ReceiveMessageAsync(object state)
{
Message m = (Message)state;if (m is CommandMessage)
OnCommandReceived(m as CommandMessage);
else if (m is ReplyMessage)
OnReplyReceived(m as ReplyMessage);
else if (m is SubscriptionMessage)
{
SubscriptionMessage sm = m as SubscriptionMessage;
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
}
}I was wondering why commands and replies were going through just fine, but subscription messages would never be routed properly.. Then I remembered what a SubscriptionMessage was...
public class SubscriptionMessage : CommandMessage
Sometimes, order matters :)
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)Unrelated to the topic at hand, you could just perform the 'as' operation once and save it off in a variable to perform fewer type castings.
private void ReceiveMessageAsync(object state)
{
SubscriptionMessage sm;
CommandMessage cm;
ReplyMessage rm;if ((sm = (state as SubscriptionMessage)) != null)
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
else if ((cm = (state as CommandMessage)) != null)
OnCommandReceived(cm);
else if ((rm = (state as ReplyMessage)) != null)
OnReplyReceived(rm);
} -
Unrelated to the topic at hand, you could just perform the 'as' operation once and save it off in a variable to perform fewer type castings.
private void ReceiveMessageAsync(object state)
{
SubscriptionMessage sm;
CommandMessage cm;
ReplyMessage rm;if ((sm = (state as SubscriptionMessage)) != null)
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
else if ((cm = (state as CommandMessage)) != null)
OnCommandReceived(cm);
else if ((rm = (state as ReplyMessage)) != null)
OnReplyReceived(rm);
}Considered that, but I did some checking around and found that "as" takes a bit more CPU time than "is", so it would be faster for the first branch of the if, and slower for the others.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels) -
Didn't hold me up for very long, but gave me a chuckle... This is a piece of a messaging system, where the automated clients route incoming messages to more specific handlers, based on what subclass it is...
private void ReceiveMessageAsync(object state)
{
Message m = (Message)state;if (m is CommandMessage)
OnCommandReceived(m as CommandMessage);
else if (m is ReplyMessage)
OnReplyReceived(m as ReplyMessage);
else if (m is SubscriptionMessage)
{
SubscriptionMessage sm = m as SubscriptionMessage;
OnSubscriptionUpdate(sm.Source, sm.Topic, sm.Item);
}
}I was wondering why commands and replies were going through just fine, but subscription messages would never be routed properly.. Then I remembered what a SubscriptionMessage was...
public class SubscriptionMessage : CommandMessage
Sometimes, order matters :)
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)With some more analysis, would it be possible to use virtual functions to take care of this? :)
Chris Meech I am Canadian. [heard in a local bar] In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
-
Considered that, but I did some checking around and found that "as" takes a bit more CPU time than "is", so it would be faster for the first branch of the if, and slower for the others.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)Just doing my own comparisons using:
private const int COUNT = 100000000;
public class Base { }
public class Derived : Base { }static void Main(string[] args)
{
object o = new Derived();
Derived result;
bool b;Stopwatch sw; sw = Stopwatch.StartNew(); for (int i = 0; i < COUNT; ) { if ((result = (o as Derived)) != null) { ++i; } } sw.Stop(); Console.WriteLine("'As' Elapsed: {0}", sw.ElapsedMilliseconds); sw = Stopwatch.StartNew(); for (int i = 0; i < COUNT; ) { if (o is Derived) ++i; } sw.Stop(); Console.WriteLine("'Is' Elapsed: {0}", sw.ElapsedMilliseconds); Console.ReadLine();
}
I get the following results:
'As' Elapsed: 460
'Is' Elapsed: 414So, basically, it boils down to a 10% savings in time (ignoring the two operations are not on the same footing - 'as' requires an additional command to store the value in result). But because you spend additional time retyping using the 'as', you end up always spending more time. In essence, you might get into the correct branch to know the type, but converting the type blows away all the savings you made. The only case where you end up being faster is when you don't go through any branches and just bail out of the method entirely, so it depends on your usage. At the very least, you could remove the additional type cast into
Message
that is not needed. Ultimately, it is your code. Good read. :) -
With some more analysis, would it be possible to use virtual functions to take care of this? :)
Chris Meech I am Canadian. [heard in a local bar] In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
Nah, my Message classes are DataContract objects... Don't want anything but basic accessors and mutators in there. And besides, all of those OnXMessage functions ARE virtual... This is the base class for different types of automated clients... The individual jobs just override the handler for the messages they want to react to.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels) -
Just doing my own comparisons using:
private const int COUNT = 100000000;
public class Base { }
public class Derived : Base { }static void Main(string[] args)
{
object o = new Derived();
Derived result;
bool b;Stopwatch sw; sw = Stopwatch.StartNew(); for (int i = 0; i < COUNT; ) { if ((result = (o as Derived)) != null) { ++i; } } sw.Stop(); Console.WriteLine("'As' Elapsed: {0}", sw.ElapsedMilliseconds); sw = Stopwatch.StartNew(); for (int i = 0; i < COUNT; ) { if (o is Derived) ++i; } sw.Stop(); Console.WriteLine("'Is' Elapsed: {0}", sw.ElapsedMilliseconds); Console.ReadLine();
}
I get the following results:
'As' Elapsed: 460
'Is' Elapsed: 414So, basically, it boils down to a 10% savings in time (ignoring the two operations are not on the same footing - 'as' requires an additional command to store the value in result). But because you spend additional time retyping using the 'as', you end up always spending more time. In essence, you might get into the correct branch to know the type, but converting the type blows away all the savings you made. The only case where you end up being faster is when you don't go through any branches and just bail out of the method entirely, so it depends on your usage. At the very least, you could remove the additional type cast into
Message
that is not needed. Ultimately, it is your code. Good read. :)Ok, you inspired me to do some benchmarking of my own... Didn't think your test was quite accurate, as the declarations would have to be inside the loop, not outside... Didn't make much of a difference, if any. Then I tried adding two more classes (Derived2, Derived3), declaring a variable of each, testing each in turn, and doing a single "as" in the second part (After it determines the type)... Basically matching the real situation perfectly. Test object is of type 'Derived': 'As' Elapsed: 453 'Is' Elapsed: 577 Test object is of type 'Derived2' (Second test): 'As' Elapsed: 982 'Is' Elapsed: 1030 Test object is of type 'Derived3': 'As' Elapsed: 1575 'Is' Elapsed: 1529 So there is a SLIGHT gain for using the "is" strategy when testing three or more times, but since the tests would be arranged such that the more common ones are near the top, the numbers to look at would be the middle/average, in this case 'Derived2'... And your method comes out ahead :) Gotta send a note to myself to swap it over when I'm back in the office on Monday. (Granted, each of these messages is coming across a network connection, so 50ms per 100 million messages is completely insignificant, but it's the principle of the thing :) )
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels) -
Ok, you inspired me to do some benchmarking of my own... Didn't think your test was quite accurate, as the declarations would have to be inside the loop, not outside... Didn't make much of a difference, if any. Then I tried adding two more classes (Derived2, Derived3), declaring a variable of each, testing each in turn, and doing a single "as" in the second part (After it determines the type)... Basically matching the real situation perfectly. Test object is of type 'Derived': 'As' Elapsed: 453 'Is' Elapsed: 577 Test object is of type 'Derived2' (Second test): 'As' Elapsed: 982 'Is' Elapsed: 1030 Test object is of type 'Derived3': 'As' Elapsed: 1575 'Is' Elapsed: 1529 So there is a SLIGHT gain for using the "is" strategy when testing three or more times, but since the tests would be arranged such that the more common ones are near the top, the numbers to look at would be the middle/average, in this case 'Derived2'... And your method comes out ahead :) Gotta send a note to myself to swap it over when I'm back in the office on Monday. (Granted, each of these messages is coming across a network connection, so 50ms per 100 million messages is completely insignificant, but it's the principle of the thing :) )
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)Ian Shlasko wrote:
Granted, each of these messages is coming across a network connection, so 50ms per 100 million messages is completely insignificant, but it's the principle of the thing
I completely disagree, benchmarking and optimising such a case is a complete waste of time, instead look for places where your application could really do with optimisation, work on other features, etc. While I generally think that Hoare's classic "Premature Optimisation is the Root of All Evil" is too often used to excuse developers from good design (not all optimisation is premature), this may be a classic case where the maxim really applies.
-
Ian Shlasko wrote:
Granted, each of these messages is coming across a network connection, so 50ms per 100 million messages is completely insignificant, but it's the principle of the thing
I completely disagree, benchmarking and optimising such a case is a complete waste of time, instead look for places where your application could really do with optimisation, work on other features, etc. While I generally think that Hoare's classic "Premature Optimisation is the Root of All Evil" is too often used to excuse developers from good design (not all optimisation is premature), this may be a classic case where the maxim really applies.
Hey, I didn't spend much time on it in actual development... But it's an interesting discussion point, in case next time a similar situation pops up, it actually IS important. I have another routine in a related class that actually handles the routing of messages to/from the different clients and server-side workers... That one sometimes processes 200-300 messages a second (Update notifications and logs, mostly). If that one was doing any sort of casting, something like this might be significant. So sure, shaving a couple cycles in this particular block of code is insignificant, but it's something to know for the future.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels) -
Hey, I didn't spend much time on it in actual development... But it's an interesting discussion point, in case next time a similar situation pops up, it actually IS important. I have another routine in a related class that actually handles the routing of messages to/from the different clients and server-side workers... That one sometimes processes 200-300 messages a second (Update notifications and logs, mostly). If that one was doing any sort of casting, something like this might be significant. So sure, shaving a couple cycles in this particular block of code is insignificant, but it's something to know for the future.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)All calls to reflection are slow in general. In such case, I prefer adding an additional, "constant", enumerated property "type". It is much faster to do a single integer comparison than using the reflection to determine an object's type (which is done in both "is" and "as" operators).
enum MessageTypes { Command, Reply, Subscription }
class Message {
public abstract MessageTypes MessageType { get; };
(...)
}if (msg.MessageType == MessageTypes.Command) {
}
else if (...)Greetings - Jacek