Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:77079
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: softfail (pb1.pair.com: domain realplain.com does not designate 216.33.127.82 as permitted sender)
Message-ID: <A8C874B472FF4F70B3A3E204C2979FF6@pc1>
To: "Anatol Belski" <anatol.php@belski.net>
Cc: <internals@lists.php.net>,
	"Stas Malyshev" <smalyshev@sugarcrm.com>,
	"Pierre Joye" <pierre.php@gmail.com>
References: <A85A75E7745344EDB50D308F48D2F2D5@pc1>    <5404DBD1.8050604@sugarcrm.com>    <687BE274F5C24DE1A42E3484C7E52B6C@pc1> <49fb94d5e1c8626544d7235492d9ae0b.squirrel@webmail.klapt.com>
Date: Fri, 5 Sep 2014 13:05:40 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="Windows-1252";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: [INTERNALS-WIN] [PATCH] Microsecond resolution and accuracy on Windows
From: php_lists@realplain.com ("Matt Wilmas")

Hi Anatol,

----- Original Message -----
From: "Anatol Belski"
Sent: Tuesday, September 02, 2014

> Unfortunately that's not a PR so I cannot comment there directly, so I'd
> leave a couple of the comments to the code here:

It looks like there can be "commit comments?" *shrug*  In the future, should 
a Pull Request usually be created?  I didn't know if that sends some sort of 
"Hey, add this!" notification, or simply makes it show up as a PR.

> - GetSystemTimeAdjustment can be disabled by some other app, so it should
> be called each time when gettimeofday() is called

I don't know what you mean by "disabled."  You mean the adjustment changing? 
I sort of already alluded to not thinking it's necessary to check it each 
time gettimeofday() is called -- I was already thinking of a much more 
complicated method last year involving that stuff, before finding this 
spring that Vista/7 aren't very finely adjustable like XP regarding 
*setting* SystemTimeAdjustment...

> - GetSystemTimeAdjustment could fail, so return value should have be 
> checked

I know it could fail (though not sure why), technically, but I don't know a 
reason for checking.  time_update_rate is already initialized in time.c.

> - global variables should be moved into the globals struct

Why?  Just because?  I thought it's fine to use true globals when it's safe 
to (I've seen it in another place), like function pointers.  No threading 
issues when they're only modified during startup (1 thread).

> Matt, I really think it's too short in time to take this in. I would
> suggest to leave this as is for 5.4 and 5.5 and stabilize to suggest to
> 5.6.1 or master after the good testing. It were probably applicable to
> win7 then.

I was glad to see Stas' reply to "let's see for 5.4" to, well, see if 
there's any other problems found. :-)  There's that uniqid() bug/issue like 
I said.

5.4 is also the last "official" release (with PGO) for Windows XP.

> Thanks
>
> Anatol

Thanks again for feedback,
Matt