MySQL Bugs: #33748: my_getsystime, my_micro_time and my_micro_time_and

Bug #33748	my_getsystime, my_micro_time and my_micro_time_and_time broken on windows
Submitted:	8 Jan 2008 18:18	Modified:	12 Sep 2008 15:30
Reporter:	Magnus Blåudd	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: General	Severity:	S3 (Non-critical)
Version:	5.1.23-rc	OS:	Windows
Assigned to:	Magnus Blåudd	CPU Architecture:	Any

Description:
The functions 'my_getsystime' always return 0 on windows due to a problem with checking return value from QueryPerformanceCounter that is used during initialization.

There seems like there is also a problem with that the initialization actually never runs unless you have the registry key "HKEY_LOCAL_MACHINES\software\MySQL"

This affects:
 - UUID() function, which uses my_get_systime. You get a uuid, but it's not nearly as unique it should be

The functions 'my_micro_time' and 'my_micro_time_and_time' are also affected but they fall back to use GetTickCount() that returns value in milliseconds.

The datatype used for storing the values from QueryPerformanceFrequency is ulonglong, I suggest changing it to the system's builtin LARGEINTEGER to avoid many casts back and forth.

(it also affect the time printout in mysqltest, but that is nothing to worry about.)

How to repeat:
assert(my_getsystime() > 0);

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/40778

ChangeSet@1.2686, 2008-01-09 19:13:07+01:00, msvensson@pilot.mysql.com +1 -0
  Bug#33748 my_getsystime, my_micro_time and my_micro_time_and_time broken on windows
   - Fix 'my_win_init' code that handles initialization of
     "query_performance_counter" and conversion from registry values to
     environment strings

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/42286

ChangeSet@1.2680, 2008-02-14 16:11:28+01:00, msvensson@pilot.mysql.com +2 -0
  Bug#33748 my_getsystime, my_micro_time and my_micro_time_and_time
  broken on windows(patch2)
   - Change to use 'GetSystemTimAsFileTime' when measuring time between
     to points in time
   - Update mysqltest to use the new function 'my_micro_time'

Hi Magnus,
2 small comments

>mysys/my_getsystime.c@stripped, 2008-02-14 16:11:25+01:00, msvensson@stripped +5 -21
>    Use 'GetSystemTimeAsFileTime' to imeplement 'my_micro_time' and

typo(implement)

micros_time_and_time()

>+  newtime/= 10;
>   (void) time(time_arg);

If performance of this function is a concern, call to time() can be replaced like this
if(time_arg)
  *time_arg= (time_t)((newtime - 116444736000000000LL)/10000000);

The big number is difference between Windows epoch(1. Jan 1601) to Unix epoch (1. Jan 1970), measured in 100ns units 

Otherwise ok.

Thanks,
Vlad

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/42300

ChangeSet@1.2680, 2008-02-14 19:36:36+01:00, msvensson@pilot.mysql.com +4 -0
  Bug#33748 my_getsystime, my_micro_time and my_micro_time_and_time
  broken on windows(patch2)
   - Change to use 'GetSystemTimAsFileTime' when measuring time between
     to points in time
   - Update mysqltest to use the new function 'my_micro_time'

The precision of GetSystemTimeAsFileTime() is spurious,
its accuracy is no better than that of GetTickCount().
I've seen that myself, and others have written about it, e.g.
http://blogs.msdn.com/oldnewthing/archive/2005/09/02/459952.aspx

So I would expect, but have not tested, that it's possible
on some machines to call GetSystemTimeAsFileTime() twice and
get the same result both times.

The 'GetSystemTimeAsFileTime' function is much faster to call and both it's precision and accuracy should be enough for the cases for how we use 'my_micro_time' and 'my_micro_time_and_time' in the server.

But maybe we should keep the use of 'QueryPerformanceCounter' in my_getsystime? We use that for generating UUID's (although there is protection in 'Item_func_uuid::val_str' against low res system clocks) and we want each UUID to be different.

Measurements on my machine shows that QueryPerformanceCounter takes in average(10000 samples) 4851.2169 ticks while GetSystemTimeAsFileTime uses 23.0175.

Magnus, you're right. UUID should use QueryPerformanceCounter. It needs a function with at least 100ns resolution. Performance is secondary (and doesn't need to be faster than 100ns anyway).

Also check this article http://www.ddj.com/windows/184416651

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/42372

ChangeSet@1.2680, 2008-02-15 17:17:12+01:00, msvensson@pilot.mysql.com +2 -0
  Bug#33748 my_getsystime, my_micro_time and my_micro_time_and_time
  broken on windows(patch2)
   - Change to use 'GetSystemTimAsFileTime' when measuring time between
     to points in time
   - Update mysqltest to use the new function 'my_micro_time'

If higher resolution and lower overhead were necessities
for UUID, then a cycle counter would be appropriate.
I'm not proposing that as a fix for a minor 5.1 bug,
but hope we'll get there eventually.

Pushed in 5.1.28 and 6.0.7-alpha

Noted in 5.1.28, 6.0.7 changelogs.

The internal functions my_getsystime(), my_micro_time(), and
my_micro_time_and_time() did not work correctly on Windows. One
symptom was that uniqueness of UUID() values could be compromised.

Pushed into 6.0.10-alpha (revid:luis.soares@sun.com-20090129165607-wiskabxm948yx463) (version source revid:luis.soares@sun.com-20090129163120-e2ntks4wgpqde6zt) (merge vers: 6.0.10-alpha) (pib:6)

Pushed into 5.1.32 (revid:luis.soares@sun.com-20090129165946-d6jnnfqfokuzr09y) (version source revid:sp1r-msvensson@pilot.mysql.com-20080325174049-18586) (merge vers: 5.1.24-rc) (pib:6)

Pushed into 5.1.32-ndb-6.3.23 (revid:tomas.ulin@sun.com-20090217131017-6u8qz1edkjfiobef) (version source revid:tomas.ulin@sun.com-20090203133556-9rclp06ol19bmzs4) (merge vers: 5.1.32-ndb-6.3.22) (pib:6)

Pushed into 5.1.32-ndb-6.4.3 (revid:tomas.ulin@sun.com-20090217134419-5ha6xg4dpedrbmau) (version source revid:tomas.ulin@sun.com-20090203133556-9rclp06ol19bmzs4) (merge vers: 5.1.32-ndb-6.3.22) (pib:6)

Pushed into 5.1.32-ndb-6.2.17 (revid:tomas.ulin@sun.com-20090217134216-5699eq74ws4oxa0j) (version source revid:tomas.ulin@sun.com-20090201210519-vehobc4sy3g9s38e) (merge vers: 5.1.32-ndb-6.2.17) (pib:6)