Re: [PHP-DEV] Re: [INTERNALS-WIN] [PATCH] Microsecond resolution and accuracy on Windows

[email protected] ("Matt Wilmas")
Newsgroups php.internals
Message-ID <A8C874B472FF4F70B3A3E204C2979FF6@pc1>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.