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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
