Re: [Quickfix-developers] UtcTimeStamp implementation bugs

[Oren M. <or...@qu...>]

I would be interested in seeing the patch.  I've never been comfortable with 
the current implementation.

--oren

----- Original Message ----- 
From: "Caleb Epstein" <cal...@gm...>
To: <qui...@li...>
Sent: Monday, July 11, 2005 5:38 PM
Subject: [Quickfix-developers] UtcTimeStamp implementation bugs


QuickFIX Documentation: 
http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX FAQ: http://www.quickfixengine.org/wikifix/index.php?QuickFixFAQ
QuickFIX Support: http://www.quickfixengine.org/services.html

Internally the FIX::UtcTimeStamp class is a "struct tm", a structure
that is generally intended to store times specified in your local
timezone.  By this I mean that if the structure has tm_hour = 10,
thats 10 AM in your local timezone, not in GMT.

Because FIX uses UTC time for everything, QuickFIX populates the
members of these structure with their UTC values, so tm_hour = 10
means 10 AM in UTC.  This is all well and good, but will break on
daylight savings time boundaries when coded as it is below (see my
comments):

  UtcTimeStamp( int hour, int minute, int second,
                int date, int month, int year )
  {
    // This is redundant and should be removed for efficiency
    setCurrent();
    setHour( hour );
    setMinute( minute );
    setSecond( second );
    setMillisecond( 0 );
    setDate( date );
    setMonth( month );
    setYear( year );
    // mktime converts a struct tm (IN LOCAL TIME) to a time_t
    time_t t = mktime( (tm*)this );
    // Converts a time_t back to a struct tm.  Presumably this is done
    // for normalization so that a user specifying a date of 31-02-2005
    // will end up with a more correct 02-03-2005?
    *static_cast<tm*>(this) = time_localtime( &t );
    tm_isdst = -1;
  }

Where this code breaks is when you pass in a time that falls precisely
on a daylight savings time change-over.  For example, if you take the
*UTC* time 2005-04-03 2:00 AM and run it through this transformation,
here's what you get:

1. Intiialize struct tm: tm_year = 105, tm_mon = 3, tm_mday = 3,
tm_hour = 2, tm_min = 0, tm_sec = 0, tm_isdst = -1
2. Call mktime, yielding result = 1112511600 (given my local US/Eastern 
time)
3. Call localtime on this value, yielding: tm_year = 105, tm_mon = 3,
tm_mday = 3, tm_hour = *3*, tm_min = 0, tm_sec = 0, tm_isdst = 1

So we've actually turned 2 AM into 3 AM and we can't reverse this.

My impression is that the mktime and localtime calls (the latter of
which is redundant - POSIX mktime modifies the passed-in struct tm in
place so you don't need to call localtime at all) are used only to
normalize values that might be specified incorrectly by the user (e.g.
February 30th or the like)

But, because of their localtime semantics and the way that QuickFIX
populates the structures with GMT values, they actually cause breakage
in certain edge situations.  These functions are also performance pigs
on some platforms (notably Solaris before Solaris 9) where I've seen
QuickFIX spend ~50% of its runtime in mktime calls.

For these reasons, I'm going to submit a patch that drops the
mktime/localtime usage (and some unncessary setCurrent calls) from the
UtcTimeStamp class which should end up giving a small performance
boost.  Can anyone see a flaw in this analysis?

-- 
Caleb Epstein
caleb dot epstein at gmail dot com


-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
Quickfix-developers mailing list
Qui...@li...
https://lists.sourceforge.net/lists/listinfo/quickfix-developers