The Amazing Power of Really Good Design – And Hard Work
Today I was very pleased to see that I could add a second exchange feed to the codebase. Yeah... just one day. Pretty amazing. I know it's primarily due to a good design because the number of lines of code I had to write was very few - on the order of 600 lines, but there's still a little bit of good old hard work to attribute to it as well.
But really, it was the design. What a great design. This is something I'm going to enjoy over and over again as I keep working with this codebase. I need to add in at least six more feeds, but if they are only a day or two per feed, I'm still done long before I had expected to be. Amazing.
So after I had it all done, I looked at the code and realized that when I was "unpacking" the time data from the exchange into milliseconds since epoch, I was making a few system calls, and that was going to come back to bite me later as the loads got higher and higher. The original code looked like:
/* * This method takes the exchange-specific time format and converts it * into a timestamp - msec since epoch. This is necessary to parse the * timestamp out of the exchange messages as the formats are different. */ uint64_t unpackTime( const char *aCode, uint32_t aSize ) { /* * The GIDS format of time is w.r.t. midnight, and a simple, 9-byte * field: HHMMSSCCC - so we can parse out this time, but need to add * in the offset of the date if we want it w.r.t. epoch. */ uint64_t timestamp = 0; // check that we have everything we need if ((aCode == NULL) || (aSize < 9)) { cLog.warn("[unpackTime] the passed in data was NULL or insufficient " "length to do the job. Check on it."); } else { // first, get the current date/time... time_t when_t = time(NULL); struct tm when; localtime_r(&when_t, &when); // now let's overwrite the hour, min, and sec from the data when.tm_hour = (aCode[0] - '0')*10 + (aCode[1] - '0'); when.tm_min = (aCode[2] - '0')*10 + (aCode[3] - '0'); when.tm_sec = (aCode[4] - '0')*10 + (aCode[5] - '0'); // ...and yank the msec while we're at it... time_t msec = ((aCode[6] - '0')*10 + (aCode[7] - '0'))*10 + (aCode[8] - '0'); // now make the msec since epoch from the broken out time timestamp = mktime(&when) + msec; if (timestamp < 0) { // keep it to epoch - that's bad enough timestamp = 0; // ...and the log the error cLog.warn("[unpackTime] unable to create the time based on the " "provided data"); } } return timestamp; }
The problem is that there are two rather costly calls - localtime_r and mktime. They are very necessary, as the ability to calculate milliseconds since epoch is a non-trivial problem, but still... it'd be nice to not have to do that.
So I created two methods: the first was just a rename of this guy:
/* * This method takes the exchange-specific time format and converts it * into a timestamp - msec since epoch. This is necessary to parse the * timestamp out of the exchange messages as the formats are different. */ uint64_t unpackTimeFromEpoch( const char *aCode, uint32_t aSize ) { // ... }
and the second was a much more efficient calculation of the milliseconds since midnight:
/* * This method takes the exchange-specific time format and converts it * into a timestamp - msec since midnight. This is necessary to parse * the timestamp out of the exchange messages as the formats are * different. */ uint64_t unpackTimeFromMidnight( const char *aCode, uint32_t aSize ) { /* * The GIDS format of time is w.r.t. midnight, and a simple, 9-byte * field: HHMMSSCCC - so we can parse out this time. */ uint64_t timestamp = 0; // check that we have everything we need if ((aCode == NULL) || (aSize < 9)) { cLog.warn("[unpackTimeFromMidnight] the passed in data was NULL " "or insufficient length to do the job. Check on it."); } else { // now let's overwrite the hour, min, and sec from the data time_t hour = (aCode[0] - '0')*10 + (aCode[1] - '0'); time_t min = (aCode[2] - '0')*10 + (aCode[3] - '0'); time_t sec = (aCode[4] - '0')*10 + (aCode[5] - '0'); time_t msec = ((aCode[6] - '0')*10 + (aCode[7] - '0'))*10 + (aCode[8] - '0'); timestamp = ((hour*60 + min)*60 + sec)*1000 + msec; if (timestamp < 0) { // keep it to midnight - that's bad enough timestamp = 0; // ...and the log the error cLog.warn("[unpackTimeFromMidnight] unable to create the time " "based on the provided data"); } } return timestamp; }
At this point, I have something that has no system calls in it, and since I'm parsing all these exchange messages, that's going to really pay off in the end. I'm not going to have to do any nasty context switching for these calls - just simple multiplications and additions. I like being able to take the time to go back and clean this kind of stuff up. Makes me feel a lot better about the potential performance issues.
Oh... I forgot... in the rest of my code, I handled the difference in these two by looking at the magnitude of the value. Anything less than "a day" had to be "since midnight" - the rest are "since epoch". Pretty simple.
It works wonderfully!