Everything

php

Switching max_execution_time from CPU time to wall-clock time and from SIGPROF to SIGALRM

php.internals

Arnaud.lb

2 years ago

Hi internals,

There is an issue with max_execution_time on MacOS, probably only
MacOS 14 on Apple Silicon, that causes timeouts to fire too early [1].
max_execution_time is implemented with setitimer(ITIMER_PROF) on this
platform, and the SIGPROF signal is delivered too early for some
reason. Switching to ITIMER_REAL appears to fix the issue, and Manuel
Kress opened a PR [3].

Are there objections against switching to ITIMER_REAL on MacOS (Apple
Silicon only) in all currently supported PHP versions? (next 8.2.x,
8.3.x, 8.x)

Apart from fixing the issue, this would introduce the following minor
breaking changes:

- max_execution_time on MacOS would be based on wall-clock time rather
than CPU time, so the time spent in I/O, sleep(), and syscalls in
general would count towards the max_execution_time
- The SIGALRM signal would be used instead of the SIGPROF signal
(using SIGALRM conflicts with pcntl_alarm(), but SIGPROF conflicts
with profilers). As noted by Dmitry, it also conflicts with sleep() on
some platforms, however this should be safe on MacOS.

Currently max_execution_time is based on wall-clock time on ZTS and
Windows, and CPU time otherwise. On Linux and FreeBSD, wall-clock time
can also be used when building with
--enable-zend-max-execution-timers. Máté proposed to add a wall-clock
based timeout in the past [2] but the discussion has stalled. Any
thoughts about eventually switching other platforms to wall-clock
timeouts in the next 8.x ?

TL;DR:
- Any objection about using wall-clock max_execution_time and SIGALRM
on MacOS Apple Silicon in all supported versions?
- Thoughts about using wall-clock timeouts on all platforms in the next 8.x ?

[1] https://github.com/php/php-src/issues/12814
[2] https://github.com/php/php-src/pull/6504
[3] https://github.com/php/php-src/pull/13567

Best Regards,
Arnaud

Robert Landers

2 years ago

On Fri, May 17, 2024 at 3:50 PM Arnaud Le Blanc <arnaud.lb@gmail.com> wrote:

>
> Hi internals,
>
> There is an issue with max_execution_time on MacOS, probably only
> MacOS 14 on Apple Silicon, that causes timeouts to fire too early [1].
> max_execution_time is implemented with setitimer(ITIMER_PROF) on this
> platform, and the SIGPROF signal is delivered too early for some
> reason. Switching to ITIMER_REAL appears to fix the issue, and Manuel
> Kress opened a PR [3].
>
> Are there objections against switching to ITIMER_REAL on MacOS (Apple
> Silicon only) in all currently supported PHP versions? (next 8.2.x,
> 8.3.x, 8.x)
>
> Apart from fixing the issue, this would introduce the following minor
> breaking changes:
>
> - max_execution_time on MacOS would be based on wall-clock time rather
> than CPU time, so the time spent in I/O, sleep(), and syscalls in
> general would count towards the max_execution_time
> - The SIGALRM signal would be used instead of the SIGPROF signal
> (using SIGALRM conflicts with pcntl_alarm(), but SIGPROF conflicts
> with profilers). As noted by Dmitry, it also conflicts with sleep() on
> some platforms, however this should be safe on MacOS.
>
> Currently max_execution_time is based on wall-clock time on ZTS and
> Windows, and CPU time otherwise. On Linux and FreeBSD, wall-clock time
> can also be used when building with
> --enable-zend-max-execution-timers. Máté proposed to add a wall-clock
> based timeout in the past [2] but the discussion has stalled. Any
> thoughts about eventually switching other platforms to wall-clock
> timeouts in the next 8.x ?
>
> TL;DR:
> - Any objection about using wall-clock max_execution_time and SIGALRM
> on MacOS Apple Silicon in all supported versions?
> - Thoughts about using wall-clock timeouts on all platforms in the next 8.x ?
>
> [1] https://github.com/php/php-src/issues/12814
> [2] https://github.com/php/php-src/pull/6504
> [3] https://github.com/php/php-src/pull/13567
>
> Best Regards,
> Arnaud

This sounds like a bug with Mac vs. a PHP issue. That being said, the
biggest issue between changing these clocks is that ITIMER_PROF is
(practically, but not explicitly) monotonic. ITIMER_REAL can go
backwards (NTP clock adjustments) which might have interesting
side-effects if the clock is adjusted.

The problem might actually be using ITIMER_PROF, which "Measures CPU
time used by the process, including both user space and kernel space"
and usage of sockets/threads might give an "accelerated" value while
maybe ITIMER_VIRTUAL is the one we should be using since it "Measures
CPU time used by the process (user space)" which won't count kernel
timings.

Robert Landers
Software Engineer
Utrecht NL

Arnaud.lb

2 years ago

Hi Robert,

> This sounds like a bug with Mac vs. a PHP issue.

This is what I believe as well. FWIW I was able to reproduce the issue
outside of PHP [2]. I've reported the bug to Apple, but I don't expect
it to be fixed quickly, if at all.

> That being said, the
> biggest issue between changing these clocks is that ITIMER_PROF is
> (practically, but not explicitly) monotonic. ITIMER_REAL can go
> backwards (NTP clock adjustments) which might have interesting
> side-effects if the clock is adjusted.

Good point. Do you have references about ITIMER_REAL using a
non-monotonic clock, besides the lack of specification regarding the
clock? Based on experiments and the code [1], I think that ITIMER_REAL
uses a monotonic clock on MacOS. It's not ideal to rely on that, and
we should use a better specified timer mechanism if we eventually
switch to wall-clock time on all platforms, but it seems reasonable as
a workaround for the ITIMER_PROF bug on MacOS/Apple Silicon.

> The problem might actually be using ITIMER_PROF, which "Measures CPU
> time used by the process, including both user space and kernel space"
> and usage of sockets/threads might give an "accelerated" value while
> maybe ITIMER_VIRTUAL is the one we should be using since it "Measures
> CPU time used by the process (user space)" which won't count kernel
> timings.

Unfortunately ITIMER_VIRTUAL is not really useful as a
max_execution_time implementation as it will basically never fire in a
syscall-heavy workload. E.g. after replacing ITIMER_PROF by
ITIMER_VIRTUAL in [2], the program runs for well over the specified
time before receiving a signal, despite consuming a considerable
amount of resources.

[1] https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/kern_time.c#L432
[2] https://gist.github.com/arnaud-lb/012195a2fe4d3a2c1bff530a73ae6b11

Best Regards,
Arnaud

Rob Landers

2 years ago

On Tue, May 21, 2024, at 14:39, Arnaud Le Blanc wrote:

> Hi Robert,
>
> > This sounds like a bug with Mac vs. a PHP issue.
>
> This is what I believe as well. FWIW I was able to reproduce the issue
> outside of PHP [2]. I've reported the bug to Apple, but I don't expect
> it to be fixed quickly, if at all.
>
> > That being said, the
> > biggest issue between changing these clocks is that ITIMER_PROF is
> > (practically, but not explicitly) monotonic. ITIMER_REAL can go
> > backwards (NTP clock adjustments) which might have interesting
> > side-effects if the clock is adjusted.
>
> Good point. Do you have references about ITIMER_REAL using a
> non-monotonic clock, besides the lack of specification regarding the
> clock? Based on experiments and the code [1], I think that ITIMER_REAL
> uses a monotonic clock on MacOS. It's not ideal to rely on that, and
> we should use a better specified timer mechanism if we eventually
> switch to wall-clock time on all platforms, but it seems reasonable as
> a workaround for the ITIMER_PROF bug on MacOS/Apple Silicon.

That was my only concern. That being said, I’m all in favor of wall-clock time, personally. It’s much easier to reason about.

>
> > The problem might actually be using ITIMER_PROF, which "Measures CPU
> > time used by the process, including both user space and kernel space"
> > and usage of sockets/threads might give an "accelerated" value while
> > maybe ITIMER_VIRTUAL is the one we should be using since it "Measures
> > CPU time used by the process (user space)" which won't count kernel
> > timings.
>
> Unfortunately ITIMER_VIRTUAL is not really useful as a
> max_execution_time implementation as it will basically never fire in a
> syscall-heavy workload. E.g. after replacing ITIMER_PROF by
> ITIMER_VIRTUAL in [2], the program runs for well over the specified
> time before receiving a signal, despite consuming a considerable
> amount of resources.
>
> [1] https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/kern_time.c#L432
> [2] https://gist.github.com/arnaud-lb/012195a2fe4d3a2c1bff530a73ae6b11
>
> Best Regards,
> Arnaud
>

— Rob

Tim Düsterhus

2 years ago

Hi

On 5/21/24 14:39, Arnaud Le Blanc wrote:

> This is what I believe as well. FWIW I was able to reproduce the issue
> outside of PHP [2]. I've reported the bug to Apple, but I don't expect
> it to be fixed quickly, if at all.

Is the bug report public / can you provide a link to it?

Best regards
Tim Düsterhus

Arnaud.lb

2 years ago

Hi Tim,

The report submitted to Apple via Feedback Assistant is not public,
but here is a mirror:
https://openradar.appspot.com/radar?id=5583058442911744

Best regards,
Arnaud

On Thu, May 23, 2024 at 8:26 PM Tim Düsterhus <tim@bastelstu.be> wrote:

Calvin Buckley

2 years ago

On May 17, 2024, at 10:47 AM, Arnaud Le Blanc <arnaud.lb@gmail.com> wrote:

>
> Hi internals,
>
> There is an issue with max_execution_time on MacOS, probably only
> MacOS 14 on Apple Silicon, that causes timeouts to fire too early [1].
> max_execution_time is implemented with setitimer(ITIMER_PROF) on this
> platform, and the SIGPROF signal is delivered too early for some
> reason. Switching to ITIMER_REAL appears to fix the issue, and Manuel
> Kress opened a PR [3].
>
> Are there objections against switching to ITIMER_REAL on MacOS (Apple
> Silicon only) in all currently supported PHP versions? (next 8.2.x,
> 8.3.x, 8.x)
>
> Apart from fixing the issue, this would introduce the following minor
> breaking changes:
>
> - max_execution_time on MacOS would be based on wall-clock time rather
> than CPU time, so the time spent in I/O, sleep(), and syscalls in
> general would count towards the max_execution_time
> - The SIGALRM signal would be used instead of the SIGPROF signal
> (using SIGALRM conflicts with pcntl_alarm(), but SIGPROF conflicts
> with profilers). As noted by Dmitry, it also conflicts with sleep() on
> some platforms, however this should be safe on MacOS.
>
> Currently max_execution_time is based on wall-clock time on ZTS and
> Windows, and CPU time otherwise. On Linux and FreeBSD, wall-clock time
> can also be used when building with
> --enable-zend-max-execution-timers. Máté proposed to add a wall-clock
> based timeout in the past [2] but the discussion has stalled. Any
> thoughts about eventually switching other platforms to wall-clock
> timeouts in the next 8.x ?
>
> TL;DR:
> - Any objection about using wall-clock max_execution_time and SIGALRM
> on MacOS Apple Silicon in all supported versions?
> - Thoughts about using wall-clock timeouts on all platforms in the next 8.x ?
>
> [1] https://github.com/php/php-src/issues/12814
> [2] https://github.com/php/php-src/pull/6504
> [3] https://github.com/php/php-src/pull/13567
>
> Best Regards,
> Arnaud

FWIW, Cygwin and IBM i are also using wall time, because these platforms
don't do execution time. IIRC, the difference between platforms with how
max_execution_time is counted doesn't seem to be documented last I checked.

Arnaud.lb

2 years ago

Hi Calvin,

> FWIW, Cygwin and IBM i are also using wall time, because these platforms
> don't do execution time.

Thank you for the additional info.

> IIRC, the difference between platforms with how
> max_execution_time is counted doesn't seem to be documented last I checked.

Currently the max_execution_time documentation has a note about that,
but it only mentions Windows:

> https://www.php.net/manual/en/info.configuration.php#ini.max-execution-time
> On non Windows systems, the maximum execution time is not affected by system calls, stream operations etc. Please see the set_time_limit() function for more details.

Best Regards,
Arnaud

Kévin Dunglas

2 years ago

Hello,

I'm in favor of merging Arnaud's patch for macOS while waiting for a better
solution like relying on Grand Central Dispatch or another non-signal-based
solution (https://github.com/php/php-src/pull/13468), which would allow
max_execution_time to work with ZTS builds on mac as well.

I'm also in favor of using wall-clock time wherever possible (disclaimer:
I'm the original author of this feature for Linux and FreeBSD).

Best,

Benjamin Eberlei

2 years ago

On Fri, May 17, 2024 at 3:49 PM Arnaud Le Blanc <arnaud.lb@gmail.com> wrote:

> Hi internals,
>
> There is an issue with max_execution_time on MacOS, probably only
> MacOS 14 on Apple Silicon, that causes timeouts to fire too early [1].
> max_execution_time is implemented with setitimer(ITIMER_PROF) on this
> platform, and the SIGPROF signal is delivered too early for some
> reason. Switching to ITIMER_REAL appears to fix the issue, and Manuel
> Kress opened a PR [3].
>
> Are there objections against switching to ITIMER_REAL on MacOS (Apple
> Silicon only) in all currently supported PHP versions? (next 8.2.x,
> 8.3.x, 8.x)
>
> Apart from fixing the issue, this would introduce the following minor
> breaking changes:
>
> - max_execution_time on MacOS would be based on wall-clock time rather
> than CPU time, so the time spent in I/O, sleep(), and syscalls in
> general would count towards the max_execution_time
> - The SIGALRM signal would be used instead of the SIGPROF signal
> (using SIGALRM conflicts with pcntl_alarm(), but SIGPROF conflicts
> with profilers). As noted by Dmitry, it also conflicts with sleep() on
> some platforms, however this should be safe on MacOS.
>
> Currently max_execution_time is based on wall-clock time on ZTS and
> Windows, and CPU time otherwise. On Linux and FreeBSD, wall-clock time
> can also be used when building with
> --enable-zend-max-execution-timers. Máté proposed to add a wall-clock
> based timeout in the past [2] but the discussion has stalled. Any
> thoughts about eventually switching other platforms to wall-clock
> timeouts in the next 8.x ?
>
> TL;DR:
> - Any objection about using wall-clock max_execution_time and SIGALRM
> on MacOS Apple Silicon in all supported versions?
> - Thoughts about using wall-clock timeouts on all platforms in the next
> 8.x ?
>
> [1] https://github.com/php/php-src/issues/12814
> [2] https://github.com/php/php-src/pull/6504
> [3] https://github.com/php/php-src/pull/13567
>
> Best Regards,
> Arnaud
>

For reference: There is also this RFC for max_execution_wall_time that was
discussed four years ago. https://externals.io/message/112492#112492