Re: [PATCH] Microsecond resolution and accuracy on Windows

Hi Anatol! ----- Original Message ----- From: "Anatol Belski" Sent: Tuesday, September 02, 2014

Hi Matt, On Mon, September 1, 2014 20:36, Matt Wilmas wrote:

Hi all! I'm back after several years, and will have a few more changes for Windows, at least. (It was CVS back then, so I still have to figure some things out... Just had to edit files on Github site. :-/) Anyway, this patch is for microtime, etc. on Windows XP-7. In March 2013, the fix for bug #64370 reduced the resolution to 1/64 or 1/100 of a second or so -- hardly "micro" time. :*( See also bugs #64633 and #65626. The highest resolution time function available before Windows 8 has simply been used by itself since then. High-resolution Performance Counters were always used until then, but in a way that may become inaccurate and error-prone on some Windows+(virtual) hardware combinations. One problem with the long-standing previous Performance Counter implementation is that it tried to be monotonic (always increasing). But gettimeofday() IS supposed to be affected by changes in system time. Another problem was assuming that Performance Counters would track at a nearly-perfect rate with the real clock even without system time changes. The solution to have high-resolution like the past, and accuracy like now (after #64370 fix) is fairly simple and straightforward: Use Performance Counters, but check that the value is within "range" of the real, current time! With this implementation, you get high-resolution at least over short periods (e.g. when it matters) AND accuracy over a longer time between calls (e.g. won't notice few milliseconds of lost resolution), with no buggy/incorrect times. (It seems there *could* be a Counter synchronization issue between multiple cores/processors on *some* hardware on *XP*. I don't think this is a concern, nor related to previous bugs? http://msdn.microsoft.com/en-us/library/windows/desktop/dn553408(v=vs.85) . aspx#qpc_support_in_windows_versions ) It's much more optimized than what's there now, and slightly over the old implementation. Not sure if I should give the saved patch link, or the "live compare" (?) on Github, so I'll do both for now: http://realplain.com/php/microtime_5_4.diff https://github.com/matt-moo/php-src/compare/PHP-5.4.diff Against 5.4 since that's what I quickly worked on so it's ready for the next 5.4 release (Stas?). (Although I guess we're supposed to change the oldest branch usually?) What do you think? Questions or comments?

unfortunately the patch doesn't fix the both issues, a quick test shows just a return to the old behaviour:

php -n -r "$t0 = microtime(true); do { $t1 = microtime(true); echo $t1 -

$t0, PHP_EOL; } while($t1 >= $t0 && ($t0 = $t1));" .......................................... 4.7922134399414E-5 4.6968460083008E-5 4.7922134399414E-5 4.8160552978516E-5 4.7922134399414E-5 4.7922134399414E-5 4.8160552978516E-5 4.887580871582E-5 0.00020909309387207 -0.015614986419678 I took an arbitrary win7 VM for that. The error happens after a couple of seconds of running. That's just before starting to argue. We probably would get WTFs from the users which had this before and had that fixed. IMHO, it's still better not to have $t1 < $t0 even with worse accuracy.

Thanks for all the feedback! I was just waiting to get something "solid" before replying. :-) I was like, "Well, hell," when I saw this first reply. :*( BUT, I think it's fixed now! Patch updated (at links above for anyone). I just quickly implemented what I had in my mind for a while Sat. morning (half the time was trying to consider what would happen in different scenarios), so I didn't spend much time on it at all. :-) Brief checking after, but yep, I was able to reproduce your result (XP, real system) after a few minutes (15-20 max) and realized what was happening. I thought about and tried a few things in the following couple days, before settling (more "obviously" now) on what's there now ("computed" out-of-valid-range logic was too simple before, oops). Maybe THIS is actually the correct logic I had in mind last October! I haven't seen any problem with it after hours and hours (which seems right). The only time there could be t1 < t0 is if the clock is set backwards (which obviously happens now with the current low-res version).

I still think that this issue is unsolvable within PHP. Fixing this for one group of users will break it for another. There are too many factors affecting this. Besides the hardware we cannot affect, there are programs affecting the behaviour (for example some could use SetSystemTimeAdjustment()).

See if you can break/find a problem with it now, please. :-)

And that is IMO the main mistake with this approach (and the one before removing QPC) - it's that QPC is based on an independent source, while GetSystemTimeAdjustment and similar time functions are synchronizable to an external source. Haven't tried, but just asking - would it work correct on day light saving days? QPC doc also states it is dedicated for time intervals measurements. The user would think that he gets a real correct microtime timestamp, while in reality it's a "computed" one, but the equation used for the computation is far incomplete.

By "it," you mean my/any QPC implementation with DST changes? Yes, GetSystemTime... is like time() I assume and doesn't change with DST -- otherwise PHP's date stuff would be affected. And my version of course always respects the currently-reported time, anyway, unlike the OLD version.

Inbetween i was developing pecl.php.net/hrtime which uses APIs similar to QPC on linux. While testing it even on linux we have seen a shift between gettimeofday and monotonic clocks. Well, there one needs much longer time to reproduce and in reality one doesn't know which of those APIs exactly has a bug. But just as an example.

Yeah, I saw about your hrtime package in the one bug comment (interesting), but haven't looked at it yet...

At the end - I would not include this patch that fast especially shortly before the PHP-5.4 doors are closing afterwards. While it were of course amazing to have a fix, the patch doesn't seem to fix the bad accuracy but brings the old behavior back. I have also a couple of notes to the code, i'd leave my comments on the patch repo. Matt, I have anyway to thank you for the effort and energy you spent on this.

Still hoping for 5.4 for at least the "security" related view with uniqid(). :-) BTW, I also removed the useless modulo from microtime(). (The GitHub patch is breaking the uniqid.c header... ignore.)

Regards Anatol

Thanks again, Matt