Newsgroups: php.internals,php.internals.win
Path: news.php.net
Xref: news.php.net php.internals:77029 php.internals.win:1057
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error)
Message-ID: <510a8e4573dee56f3af12ce067bcc697.squirrel@webmail.klapt.com>
In-Reply-To: <A85A75E7745344EDB50D308F48D2F2D5@pc1>
References: <A85A75E7745344EDB50D308F48D2F2D5@pc1>
Date: Tue, 2 Sep 2014 10:03:08 +0200
To: "Matt Wilmas" <php_lists@realplain.com>
Cc: internals@lists.php.net,
 internals-win@lists.php.net,
 "Pierre Joye" <pierre.php@gmail.com>,
 "Anatol Belski" <anatol.php@belski.net>
User-Agent: SquirrelMail/1.5.2 [SVN]
MIME-Version: 1.0
Content-Type: text/plain;charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PATCH] Microsecond resolution and accuracy on Windows
From: anatol.php@belski.net ("Anatol Belski")

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.

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()).

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.

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.

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.

Regards

Anatol