Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115282
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.7\))
Date: Sat, 3 Jul 2021 16:03:34 +0200
References: <0BBAA2F7-02D7-4AEE-B48A-97436A0D8E5D@gmail.com>
 <DM6PR07MB66183FB2B720E47E95DA6615F9089@DM6PR07MB6618.namprd07.prod.outlook.com>
To: "internals@lists.php.net" <internals@lists.php.net>
In-Reply-To: <DM6PR07MB66183FB2B720E47E95DA6615F9089@DM6PR07MB6618.namprd07.prod.outlook.com>
Message-ID: <DAF6D39C-F3FC-407C-B331-B3AD693C481B@gmail.com>
Subject: Re: [PHP-DEV] [RFC] clamp
From: hallbergkim@gmail.com (Kim Hallberg)

> On 24 Jun 2021, at 1:50 AM, tyson andre <tysonandre775@hotmail.com> =
wrote:
>=20
> So I think the major objections are that:
>=20
> 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
>=20
>   (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 <tysonandre775@hotmail.com> =
wrote:
>=20
> 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=3D-O2`, opcache =
enabled(zend_extension=3Dopcache, =
opcache.enable=3D1,opcache.enable_cli=3D1),
> 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)
>=20
> For userland implementations that did use min/max, they probably =
weren't performance sensitive for the application.
>=20
> ```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=E2=80=99t 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 =
=E2=80=9Cefficient=E2=80=9D version in some cases and faster in others. =
That is to be expected though since the efficient version doesn=E2=80=99t =
check for a valid range, if a user mixes up the min and max values the =
efficient clamp function doesn=E2=80=99t 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=E2=80=99t 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=E2=80=99s 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=E2=80=99t =
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=E2=80=99s OT for this RFC so.

As for your last and forth point, I=E2=80=99ll 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=E2=80=99s added to value.

Thanks, Kim.