> On 24 Jun 2021, at 1:50 AM, tyson andre <tysonandre...@hotmail.com> wrote:
>
> So I think the major objections are that:
>
> 1. This is easy to implement in userland and there's negligible performance
> benefit
> (in code that is a performance sensitive loop, it may still be worse
> compared to `$num < $min ? $min : ($num > $max : $max : $num)` when validity
> of types and min/max are known, especially with the JIT)
> 2. People not being sure if they'd ever use it personally, especially with
> the ability to customize parameter order and permitted argument types in
> userland
> 3. It's inconsistent with min() and max(), which support any comparable type
> such as GMP(https://www.php.net/manual/en/class.gmp)
> (arbitrary precision numbers), DateTime, etc., and that may lead to
> surprises.
> Although PHP's comparison operator and
> https://www.php.net/manual/en/function.min.php have many, many
> inconsistencies, already
>
> (Though special casing GMP is probably a bad idea due to it being optional
> and having an incompatible license with core for packagers)
> 4. I'm not sure what this is meant to do with the float NAN (Not A Number)
> from the RFC description, but that's solvable
> On 24 Jun 2021, at 2:15 AM, tyson andre <tysonandre...@hotmail.com> wrote:
>
> I'd strongly prefer an actual benchmark for context and accuracy - is it
> actually faster or slower than the most efficient userland implementation and
> by how much?
> E.g. in an optimized NTS build with `CFLAGS=-O2`, opcache
> enabled(zend_extension=opcache, opcache.enable=1,opcache.enable_cli=1),
> and no debug configure flags, how many calls per second can be made on
> variable values of $num for both?
> (I'd assume over twice as fast as calling both min/max from another function,
> but possibly slower than efficient_clamp, but haven't run this)
>
> For userland implementations that did use min/max, they probably weren't
> performance sensitive for the application.
>
> ```php
> function efficient_clamp(int|float $num, int|float $min, int|float $max):
> int|float {
> return $num < $min ? $min : ($num > $max ? $max : $num);
> }
> ```
Will combine both emails into one here. Will tackle your first point since that
has to with performance as your second email also does.
Haven’t tested it with opcache enabled so cannot comment on the performance,
but for my general testing the internal version is more than 50% faster than
the min/max version. Slower than the “efficient” version in some cases and
faster in others. That is to be expected though since the efficient version
doesn’t check for a valid range, if a user mixes up the min and max values the
efficient clamp function doesn’t work as intended and will always return either
the min/max version and never the clamped value.
For your second point, that could be said for most internal functions,
`str_contains`, `str_starts_with` and `str_ends_with` could all three be
implemented in userland with customises parameter orders, as can most other
internals function. Frankly I don’t see this as much of an objection or issue
as a phrase used to gate keep what can become a new standardised internal
function.
Thirdly, yes I will agree it’s inconsistent with min/max functions. That is
mainly - as you stated, min/max takes mixed arguments and works with all
comparable types. Though it technically isn’t inconsistent with most userland
implementations as they mostly accept numerical values such as ints or floats
into their clamp function, meaning only ints and floats are used anyway.
Though I could see a version of clamping being useful in the `DatePeriod` class
to check if a `DateTime` is in range of a certain date range, but that’s OT for
this RFC so.
As for your last and forth point, I’ll be updating the RFC with NaN behaviour,
sticking with what Nikita suggested to throw when NaN is added to min or max,
and return NaN is it’s added to value.
Thanks, Kim.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
