Allow sleep() to accept non-integer values

  111448
August 11, 2020 07:31 vorismi3@fel.cvut.cz (=?UTF-8?Q?Michael_Vo=C5=99=C3=AD=C5=A1ek_-_=C4=8CVUT_FEL?=)
Hi everyone, 

I am the author of https://github.com/php/php-src/pull/5961 , please
provide feedback. 

All details should be in the description, also, please advise if we can
consider it as a small change not requiring RFC as Nikita proposed in
his comment. 

With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,

Michael Voříšek
  111449
August 11, 2020 08:53 rowan.collins@gmail.com (Rowan Tommins)
On Tue, 11 Aug 2020 at 08:31, Michael Voříšek - ČVUT FEL <
vorismi3@fel.cvut.cz> wrote:

> I am the author of https://github.com/php/php-src/pull/5961 , please > provide feedback. >
This idea makes a lot of sense to me as a user (I'll leave comments on the implementation to those with more C experience). I'm pretty sure I've accidentally written things like "sleep(1.5)" in the past, and think there is a strong argument for changing something here:
> Another reason is that sleep(0.1); is silently accepted now (even with strict types enabled), but the input is casted to 0 and thus producing
unexpected behaviour if the user is not aware of the current method prototype. Unless there are problems with the implementation, this seems like a straight-forward win. Regards, -- Rowan Tommins [IMSoP]
  111452
August 11, 2020 10:43 bjorn.x.larsson@telia.com (=?UTF-8?Q?Bj=c3=b6rn_Larsson?=)
Den 2020-08-11 kl. 10:53, skrev Rowan Tommins:

> On Tue, 11 Aug 2020 at 08:31, Michael Voříšek - ČVUT FEL < > vorismi3@fel.cvut.cz> wrote: > >> I am the author of https://github.com/php/php-src/pull/5961 , please >> provide feedback. >> > > This idea makes a lot of sense to me as a user (I'll leave comments on the > implementation to those with more C experience). > > I'm pretty sure I've accidentally written things like "sleep(1.5)" in the > past, and think there is a strong argument for changing something here: > >> Another reason is that sleep(0.1); is silently accepted now (even with > strict types enabled), but the input is casted to 0 and thus producing > unexpected behaviour if the user is not aware of the current method > prototype. > > Unless there are problems with the implementation, this seems like a > straight-forward win. > > Regards,
Given this unexpected behaviour, one could almost see it as a bug. I think it's worth considering if this also should be fixed in 8.0 or even earlier ;-) So good to hear the RM view on this. r//Björn L
  111650
August 20, 2020 12:15 vorismi3@fel.cvut.cz (=?UTF-8?Q?Michael_Vo=C5=99=C3=AD=C5=A1ek_-_=C4=8CVUT_FEL?=)
Hi everyone, 

thank you for your comments, based on them, I fixed these: 

- usleep is now used as a fallback as well, if interrupted, remaining
time is measured using microtime, so return value is always available 

- for BC, if not interrupted, return value remains to be 0 (integer
zero) 

Now, the sleep() function should be really universal, cross platform and
I would say also the prefered way to sleep. 

The implementaion is here https://github.com/php/php-src/pull/5961/files


Please comment on Github directly if you have any feedback left. 

> I thinkit's worth considering if this also should be fixed in 8.0 or even earlier ;-)So good to hear the RM view on this.
Sara, are you ok to include this in PHP 8.0 and do you require a RFC for it? With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, Michael Voříšek On 11 Aug 2020 12:43, Björn Larsson wrote:
> Den 2020-08-11 kl. 10:53, skrev Rowan Tommins: > > On Tue, 11 Aug 2020 at 08:31, Michael Voříšek - ČVUT FEL < > vorismi3@fel.cvut.cz> wrote: > > I am the author of https://github.com/php/php-src/pull/5961 , please > provide feedback. > > This idea makes a lot of sense to me as a user (I'll leave comments on the > implementation to those with more C experience). > > I'm pretty sure I've accidentally written things like "sleep(1.5)" in the > past, and think there is a strong argument for changing something here: > > Another reason is that sleep(0.1); is silently accepted now (even with strict types enabled), but the input is casted to 0 and thus producing > unexpected behaviour if the user is not aware of the current method > prototype. > > Unless there are problems with the implementation, this seems like a > straight-forward win. > > Regards,
Given this unexpected behaviour, one could almost see it as a bug. I think it's worth considering if this also should be fixed in 8.0 or even earlier ;-) So good to hear the RM view on this. r//Björn L
  111651
August 20, 2020 12:22 george.banyard@gmail.com ("G. P. B.")
On Thu, 20 Aug 2020 at 14:15, Michael Voříšek - ČVUT FEL <
vorismi3@fel.cvut.cz> wrote:

> Hi everyone, > > thank you for your comments, based on them, I fixed these: > > - usleep is now used as a fallback as well, if interrupted, remaining > time is measured using microtime, so return value is always available > > - for BC, if not interrupted, return value remains to be 0 (integer > zero) > > Now, the sleep() function should be really universal, cross platform and > I would say also the prefered way to sleep. > > The implementaion is here https://github.com/php/php-src/pull/5961/files > > > Please comment on Github directly if you have any feedback left. > > > I thinkit's worth considering if this also should be fixed in 8.0 or > even earlier ;-)So good to hear the RM view on this. > > Sara, are you ok to include this in PHP 8.0 and do you require a RFC for > it? > > With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, > > Michael Voříšek >
  111652
August 20, 2020 12:25 george.banyard@gmail.com ("G. P. B.")
Apologies for the double email, my client did something funcky.

On Thu, 20 Aug 2020 at 14:22, G. P. B. banyard@gmail.com> wrote:

> On Thu, 20 Aug 2020 at 14:15, Michael Voříšek - ČVUT FEL < > vorismi3@fel.cvut.cz> wrote: > >> Hi everyone, >> >> thank you for your comments, based on them, I fixed these: >> >> - usleep is now used as a fallback as well, if interrupted, remaining >> time is measured using microtime, so return value is always available >> >> - for BC, if not interrupted, return value remains to be 0 (integer >> zero) >> >> Now, the sleep() function should be really universal, cross platform and >> I would say also the prefered way to sleep. >> >> The implementaion is here https://github.com/php/php-src/pull/5961/files >> >> >> Please comment on Github directly if you have any feedback left. >> >> > I thinkit's worth considering if this also should be fixed in 8.0 or >> even earlier ;-)So good to hear the RM view on this. >> >> Sara, are you ok to include this in PHP 8.0 and do you require a RFC for >> it? >> >> With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, >> >> Michael Voříšek >> > >
Again, I personally don't understand why this could bypass the RFC process, as multiple people have already, me included, voiced their disagreement with this change. Secondly this change introduces another inconsistency, why can sleep accept a float but not usleep? If there is indeed a need for being able to specify a sleep in milliseconds I'd prefer the introduction of a msleep function then this change. Best regards George P. Banyard
  111654
August 20, 2020 12:55 vorismi3@fel.cvut.cz (=?UTF-8?Q?Michael_Vo=C5=99=C3=AD=C5=A1ek_-_=C4=8CVUT_FEL?=)
> Again, I personally don't understand why this could bypass the RFC process, as multiple people have already, me included, voiced their disagreement with this change.
This was proposed by Nikita Popov in his comment
> Secondly this change introduces another inconsistency, why can sleep accept a float but not usleep?
Nanosleep and microsleep functions are basically 1:1 of the underlaying implementation. The updated sleep() is however now never worse than the best sleep function available, thus we can use it also for time_nanosleep and usleep php function and accept float. I will implement it.
> If there is indeed a need for being able to specify a sleep in milliseconds I'd prefer the introduction of a msleep function then this change.
The issue I solve is sleep function that accepts seconds should accept floating point values as time is continous value. With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, Michael Voříšek On 20 Aug 2020 14:25, G. P. B. wrote:
> Apologies for the double email, my client did something funcky. > > On Thu, 20 Aug 2020 at 14:22, G. P. B. banyard@gmail.com> wrote: > > On Thu, 20 Aug 2020 at 14:15, Michael Voříšek - ČVUT FEL <vorismi3@fel.cvut.cz> wrote: Hi everyone, > > thank you for your comments, based on them, I fixed these: > > - usleep is now used as a fallback as well, if interrupted, remaining > time is measured using microtime, so return value is always available > > - for BC, if not interrupted, return value remains to be 0 (integer > zero) > > Now, the sleep() function should be really universal, cross platform and > I would say also the prefered way to sleep. > > The implementaion is here https://github.com/php/php-src/pull/5961/files > > Please comment on Github directly if you have any feedback left. > >> I thinkit's worth considering if this also should be fixed in 8.0 or even earlier ;-)So good to hear the RM view on this. > > Sara, are you ok to include this in PHP 8.0 and do you require a RFC for > it? > > With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, > > Michael Voříšek
Again, I personally don't understand why this could bypass the RFC process, as multiple people have already, me included, voiced their disagreement with this change. Secondly this change introduces another inconsistency, why can sleep accept a float but not usleep? If there is indeed a need for being able to specify a sleep in milliseconds I'd prefer the introduction of a msleep function then this change. Best regards George P. Banyard
  111655
August 20, 2020 13:21 carusogabriel34@gmail.com (Gabriel Caruso)
On Thu, 20 Aug 2020 at 14:55, Michael Voříšek - ČVUT FEL <
vorismi3@fel.cvut.cz> wrote:

> > Again, I personally don't understand why this could bypass the RFC > process, as multiple people have already, me included, voiced their > disagreement with this change. > > This was proposed by Nikita Popov in his comment >
Nikita is just one of the members. If other members want an RFC, we need it.. Also, the PR has a couple of :-1: votes as well, which indicates that an RFC is necessary.
> > > Secondly this change introduces another inconsistency, why can sleep > accept a float but not usleep? > > Nanosleep and microsleep functions are basically 1:1 of the underlaying > implementation. The updated sleep() is however now never worse than the > best sleep function available, thus we can use it also for > time_nanosleep and usleep php function and accept float. I will > implement it. > > > If there is indeed a need for being able to specify a sleep in > milliseconds I'd prefer the introduction of a msleep function then this > change. > > The issue I solve is sleep function that accepts seconds should accept > floating point values as time is continous value. > > With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, > > Michael Voříšek > > On 20 Aug 2020 14:25, G. P. B. wrote: > > > Apologies for the double email, my client did something funcky. > > > > On Thu, 20 Aug 2020 at 14:22, G. P. B. banyard@gmail.com> > wrote: > > > > On Thu, 20 Aug 2020 at 14:15, Michael Voříšek - ČVUT FEL < > vorismi3@fel.cvut.cz> wrote: Hi everyone, > > > > thank you for your comments, based on them, I fixed these: > > > > - usleep is now used as a fallback as well, if interrupted, remaining > > time is measured using microtime, so return value is always available > > > > - for BC, if not interrupted, return value remains to be 0 (integer > > zero) > > > > Now, the sleep() function should be really universal, cross platform and > > I would say also the prefered way to sleep. > > > > The implementaion is here https://github.com/php/php-src/pull/5961/files > > > > Please comment on Github directly if you have any feedback left. > > > >> I thinkit's worth considering if this also should be fixed in 8.0 or > even earlier ;-)So good to hear the RM view on this. > > > > Sara, are you ok to include this in PHP 8.0 and do you require a RFC for > > it? > > > > With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, > > > > Michael Voříšek > > Again, I personally don't understand why this could bypass the RFC > process, > as multiple people have already, me included, voiced their disagreement > with this change. > > Secondly this change introduces another inconsistency, why can sleep > accept a float but not usleep? > > If there is indeed a need for being able to specify a sleep in > milliseconds I'd prefer the introduction of a > msleep function then this change. > > Best regards > > George P. Banyard
  111656
August 20, 2020 13:29 kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)
> > > > Again, I personally don't understand why this could bypass the RFC > > process, as multiple people have already, me included, voiced their > > disagreement with this change. > > > > This was proposed by Nikita Popov in his comment > > > > Nikita is just one of the members. If other members want an RFC, we need > it. > > Also, the PR has a couple of :-1: votes as well, which indicates that an > RFC is necessary. >
Just for clarity, Nikita didn't propose such thing that the feature can skip the RFC process. The exact words he used is the following: This does not necessarily need an RFC, but needs a discussion on the
> mailing list at least,
as the reception here seems somewhat negative. Default target version is
> 8.1 now, though
exceptions can be made for small features. All this means is: - The discussion on the ML is a must - IPHP 8.1 is the default target Regards: Máté
  111653
August 20, 2020 12:25 carusogabriel34@gmail.com (Gabriel Caruso)
On Thu, 20 Aug 2020 at 14:16, Michael Voříšek - ČVUT FEL <
vorismi3@fel.cvut.cz> wrote:

> Hi everyone, > > thank you for your comments, based on them, I fixed these: > > - usleep is now used as a fallback as well, if interrupted, remaining > time is measured using microtime, so return value is always available > > - for BC, if not interrupted, return value remains to be 0 (integer > zero) > > Now, the sleep() function should be really universal, cross platform and > I would say also the prefered way to sleep. > > The implementaion is here https://github.com/php/php-src/pull/5961/files > > > Please comment on Github directly if you have any feedback left. > > > I thinkit's worth considering if this also should be fixed in 8.0 or > even earlier ;-)So good to hear the RM view on this. > > Sara, are you ok to include this in PHP 8.0 and do you require a RFC for > it? > > With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, > > Michael Voříšek > > On 11 Aug 2020 12:43, Björn Larsson wrote: > > > Den 2020-08-11 kl. 10:53, skrev Rowan Tommins: > > > > On Tue, 11 Aug 2020 at 08:31, Michael Voříšek - ČVUT FEL < > > vorismi3@fel.cvut.cz> wrote: > > > > I am the author of https://github.com/php/php-src/pull/5961 , please > > provide feedback. > > > > This idea makes a lot of sense to me as a user (I'll leave comments on > the > > implementation to those with more C experience). > > > > I'm pretty sure I've accidentally written things like "sleep(1.5)" in the > > past, and think there is a strong argument for changing something here: > > > > Another reason is that sleep(0.1); is silently accepted now (even with > strict types enabled), but the input is casted to 0 and thus producing > > unexpected behaviour if the user is not aware of the current method > > prototype. > > > > Unless there are problems with the implementation, this seems like a > > straight-forward win. > > > > Regards, > > Given this unexpected behaviour, one could almost see it as a bug. I > think > it's worth considering if this also should be fixed in 8.0 or even > earlier ;-) > So good to hear the RM view on this. > > r//Björn L
I don't know about Sara's opinion here, but mine, as PHP 8.0's other Release Manager, this change requires an RFC and it should target PHP 8.1. PHP 8.0 has been feature frozen for a while now.
  111461
August 11, 2020 12:34 cmbecker69@gmx.de ("Christoph M. Becker")
On 11.08.2020 at 10:53, Rowan Tommins wrote:

> Unless there are problems with the implementation, this seems like a > straight-forward win.
Not necessarily a problem, but it has to be considered that POSIX mandates that nanosleep() shall fail, if "the rqtp argument specified a nanosecond value […] greater than or equal to 1000 million."[1] Analogous for usleep() which has been removed from the POSIX standard. [1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/nanosleep.html> -- Christoph M. Becker
  111451
August 11, 2020 10:39 Danack@basereality.com (Dan Ackroyd)
Michael Voříšek wrote:

> Another reason is that sleep(0.1); is silently accepted now (even with strict types enabled),
That appears to not be true: https://3v4l.org/7YbqX Rowan wrote:
> Unless there are problems with the implementation, this seems like a straight-forward win.
From the PR.
> Implemented using nanosleep which is not guaranteed to be available everywhere.
Please just use usleep if you need more accuracy than seconds. Having a function that behaves differently based on different platforms is a bad idea. Changing a function to have surprising behaviour just to avoid using a different function, that is already available, is a really bad tradeoff. cheers Dan Ack
  111453
August 11, 2020 11:03 vorismi3@fel.cvut.cz (=?UTF-8?Q?Michael_Vo=C5=99=C3=AD=C5=A1ek_-_=C4=8CVUT_FEL?=)
> Another reason is that sleep(0.1); is silently accepted now (even with strict types enabled), > > That appears to not be true: https://3v4l.org/7YbqX
corrected, should be "without strict types enabled" - https://3v4l.org/A2olN "even with strict type enabled" statement in BC section remains valid
> Having a function that behaves differently based on different platforms is a bad idea.
I will implement fallback to usleep. I have not noticed any issues with usleep availability. Is there any known platform without nanosleep neither usleep support? With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, Michael Voříšek On 11 Aug 2020 12:39, Dan Ackroyd wrote:
> Michael Voříšek wrote: > >> Another reason is that sleep(0.1); is silently accepted now (even with > strict types enabled), > > That appears to not be true: https://3v4l.org/7YbqX > > Rowan wrote: > >> Unless there are problems with the implementation, this seems like a > straight-forward win. > > From the PR. > >> Implemented using nanosleep which is not guaranteed to be available everywhere. > > Please just use usleep if you need more accuracy than seconds. > > Having a function that behaves differently based on different > platforms is a bad idea. > > Changing a function to have surprising behaviour just to avoid using a > different function, that is already available, is a really bad > tradeoff. > > cheers > Dan > Ack
  111455
August 11, 2020 11:27 george.banyard@gmail.com ("G. P. B.")
On Tue, 11 Aug 2020 at 13:03, Michael Voříšek - ČVUT FEL <
vorismi3@fel.cvut.cz> wrote:

> > Another reason is that sleep(0.1); is silently accepted now (even with > strict types enabled), > > > > That appears to not be true: https://3v4l.org/7YbqX > > corrected, should be "without strict types enabled" - > https://3v4l.org/A2olN > > "even with strict type enabled" statement in BC section remains valid >
That's a normal type juggling behaviour of PHP to silently truncate float to integers. Maybe something that we should change by emitting a E_WARNING but that would need an RFC. Therefore I don't see why sleep should get special treatment. I also echo Dan's concerns and I find this change all in all negative. Now we need to deal with IEEE 754 floating point semantics and converting "manually" to an integer range to pass it to functions which might not be available on the platform and then still end up using sleep(0) on the C level. I'd rather see the failure condition changed to reject 0 or add a E_NOTICE/E_WARNING if 0 is passed as this has questionable semantics. Best regards George P. Banyard
  111458
August 11, 2020 12:13 Danack@basereality.com (Dan Ackroyd)
Björn Larsson wrote:
> Given this unexpected behaviour, one could almost see it as a bug.
This isn't a suddenly noticed new bug. That code has worked like that since the sleep function was committed twenty-two years ago or for five years since the release of PHP 7 and the weak/strict RFC continued the "if you don't want float -> int conversions, use strict mode" choice. There are quite a few functions in PHP that take int parameters, that have this behaviour. These could all be 'fixed' by people using strict mode, and you could make a good argument for making strict mode the default, but it's probably a bit late in the 8.0 release cycle for that, and it _might_ be a little contentious. I agree that the PHP standard library is not as good as it could be, but tweaking it piece-by-piece is not a good way of fixing it. Doing it like that would lead to subtle BC breaks that are not obvious. G.P.B. wrote:
> I'd rather see the failure condition changed to reject 0 or add a > E_NOTICE/E_WARNING if 0 is passed as this has questionable semantics.
sleep(0) is a useful thing to do in some circumstances. It has the same effect as Thread.yield() in Java. Or in English, it allows a process to say "Hello OS scheduler, I don't want to sleep right now, but if there are any other threads that are waiting for their turn, now is a great time to suspend my execution, as I'm not in the middle of anything, otherwise I'll keep processing." Again, yes we need a better way of evolving the core libraries. But tweaking it on a function by function basis is not a good way of doing that. cheers Dan Ack
  111454
August 11, 2020 11:14 php@beccati.com (Matteo Beccati)
Hi Michael,

On 11/08/2020 12:39, Dan Ackroyd wrote:
> Changing a function to have surprising behaviour just to avoid using a > different function, that is already available, is a really bad > tradeoff.
I agree. function mysleep(float $s): void { usleep($s * 1000000); } You're welcome ;-) Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/
  111511
August 14, 2020 05:30 twose@qq.com (twosee)
> 2020年8月11日 下午6:39,Dan Ackroyd <Danack@basereality.com> 写道: > > Michael Voříšek wrote: > >> Another reason is that sleep(0.1); is silently accepted now (even with > strict types enabled), > > That appears to not be true: https://3v4l.org/7YbqX > > Rowan wrote: >> Unless there are problems with the implementation, this seems like a > straight-forward win. > > From the PR. >> Implemented using nanosleep which is not guaranteed to be available everywhere. > > Please just use usleep if you need more accuracy than seconds. > > Having a function that behaves differently based on different > platforms is a bad idea. > > Changing a function to have surprising behaviour just to avoid using a > different function, that is already available, is a really bad > tradeoff. > > cheers > Dan > Ack > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
+1 I would rather have a new function like this (actually I am using it on my project): ``` function msleep(int $milli_seconds) { } ``` int is always a better choice than float, otherwise, we have to do dirty conversions and more checks on it And usually, we don't need high precision, millisecond sleep is enough, e.g. the timeout feature provided by epoll, it is the reason why many async libraries provide timeout feature in milliseconds. Regards, Twosee