Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:127498 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by lists.php.net (Postfix) with ESMTPS id 4B29B1A00BC for ; Wed, 28 May 2025 14:52:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1748443840; bh=/mIB0QBGU4ClRGwG08fKALeQQlpXOQB4KIOQ0vlLdkg=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=KrgHUCADiF/48jGMT33xVF7UCottH3U5G6rUoc4Cg3mlTc5YJpwYUFfY9tWPyzRzg xfQuMf/l64kFULgMXiMNuXq6zZSCb+4hdUy581C0/wpyIWc2qhPJAVms0MjnV+pjwU 50d9xJyP7IKNiX6s9UdCgdeeUbZxpTRbKKGpNJxNC/MeFIFzp2iRyuiqe8oZB8eRle uCWlKXSk+9HM0lvPlh+Ka2MSoMKZtFpKP++olF8bDcHN2RDnbkJtZXZqu85//YTZrs oHD795c95OrUBdvngJY14Z42WFfbEGy+WpKgm0lRyxPtJnZKivwO54nvRm6yHOihCF +OGU1unSvo9Hw== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 957F41801E6 for ; Wed, 28 May 2025 14:50:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=0.6 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_MISSING,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=4.0.1 X-Spam-Virus: Error (Cannot connect to unix socket '/var/run/clamav/clamd.ctl': connect: Connection refused) X-Envelope-From: Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 28 May 2025 14:50:39 +0000 (UTC) Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-acae7e7587dso642938166b.2 for ; Wed, 28 May 2025 07:52:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tideways-gmbh.com; s=google; t=1748443964; x=1749048764; darn=lists.php.net; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=y+WE27el/TvSOumdmeLhJfzLjqVXsO11kw696LlUItE=; b=bG3ZQytVamYN8urVRgFr2hAHX/vsn/aJb+e3UAEXxqSXaIevprLiBZXQ66JVFGkVKf 3ui4r8PCPXG9BQk68W1O7Pd7Fu66sBS+DtwZJ0/ry3Jg1MuczyBq//7NHdCtqzzCuN6+ Xfv15PvXZPQr4uCmKc3VmK/NBFlNraQG5JTc5y606H6bnBNi2Bf5APhSqBPVPJZjE8rR LrUvVWbxybAld0viYHuswvSTmXv3ENgFnGincqQ5eJrBxavOIW+rg/mqxphubqjYgPka LOVveFOoBAf0vK8RzaUXeyBy6DbM/ZvhKMR4nMfHlSMzAgPU0m1u991dHJAsHT2cJ8Qt tHuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1748443964; x=1749048764; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=y+WE27el/TvSOumdmeLhJfzLjqVXsO11kw696LlUItE=; b=H8VmYfQuF7fcDasnSXGkSM0oVKoW+sIksGfhmRLPoXLSMW9OZ6zXHsCj03awd9G/jc QFq32NlHQ/bTqDZkoJSKRHe63oucTCw8LI56IfQ/I0psZ8PqnPz8VR1Z7PGcYiUYG/xF oVQgm3KcVh65H1cvFG1TBpkW2lO98/ZylZCCVGFotNE/At/tsmKCWie/BCcvQnacL0+X 1KaeLZjhQBXCrVSibScDnvuHCXu73pFMBUuo6PvFkwWADX+jCwYqL4ZxN9oTlRk7qjaC qFdefnht/KwXfMUYl0FgxecQHABtMpqZiw8wQSwYV8CftwHNXGTiA/INEC2YTRrqaSGO MY8g== X-Gm-Message-State: AOJu0Yz75hu5ygYQMysOR+Rz6rQQ22RgQYxBqRjGxKcd5+5FrMCZtP5t 64YOB+84dA48lezH+74Ra9Jt5s3VV5LIBOmQO6Qd5CWU13zapU/UDKFbG+8OOlDqJ/3Io2V0tEq 88Bz0TBpBblrl41Y9QA7/y4R/jRm3WjXV+SRCJ3i63nRg7QfHrxlXJM4= X-Gm-Gg: ASbGncskTelNjSXkgC+0Bqr4oXITNjy0hC58t/RYM8ijSH+h/Di0/ZW9tpSHpwpPqVs 1+9/fHEAKtLWU5F58XxP/Bt8vH7A1w7UcGnSAzlx9rdfDZX5LiC5SlcWI+WLAKiEb27LEFbrXty QAA8XRJ4o9wVJ0GjPHH/G89qhf9K3F7MQszDrmGB1OkS4Ul9rDYMYIUk5Ripsnbqw= X-Google-Smtp-Source: AGHT+IHi1KH/5TP91DQkjag3nRLFAF7EEV+7Ay4Eji/US8IWsyG0G4pBWEBMAeNAeJj7VtEgvKGtmqVfJq+fnjY2d34= X-Received: by 2002:a17:907:26c7:b0:ad8:50c3:3359 with SMTP id a640c23a62f3a-ad85b03b8eemr1217988166b.9.1748443964303; Wed, 28 May 2025 07:52:44 -0700 (PDT) Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow MIME-Version: 1.0 References: In-Reply-To: Date: Wed, 28 May 2025 16:52:32 +0200 X-Gm-Features: AX0GCFtKTe1LLPsTLVolij7SmuCWanL7tmHwKxO1Nf1UEVuSjDk40uzUjPQXcok Message-ID: Subject: Re: [PHP-DEV] Re: [RFC] Clone with v2 To: Nicolas Grekas Cc: php internals , =?UTF-8?Q?Tim_D=C3=BCsterhus?= Content-Type: multipart/alternative; boundary="000000000000670a870636335521" From: volker@tideways-gmbh.com (Volker Dusch) --000000000000670a870636335521 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Nicolas, Thank you for the great email and for thinking this through. Getting input from the folks that maybe will use this feature the most is very valuable, and I appreciate that you're taking the time to do this early. On Mon, May 26, 2025 at 4:37=E2=80=AFPM Nicolas Grekas wrote: > - To me, the main technical drawback of the RFC is that it builds on > mandatory deep-cloning, which means wasted CPU/mem resources by design. > This has to be mentioned in the RFC IMHO so that voters can make an > informed decision. > > About this last point, it's the one that makes me undecided about my > voting decision on the RFC. > I shared my concern and a solution in another thread, where I also addres= s > the BC concern that is mentioned in the RFC. Since this concern has been > addressed, I think this should be considered more closely. As is, that's = a > big omission to me - recognizing and addressing the issue with deep-cloni= ng. > Quoting from your other email: > Allow to skip deep-cloning of already updated properties (that's a significant drawback of the current proposal - deep cloning before setting is a potential perf/mem hog built into the engine) : guarding deep-cloning with a strict comparison would be enough. To make sure we're on the same page: This would only come into effect if both a __clone method exists and clone($thing, $withProperties) is used. - We decided to __clone before updating properties to avoid BC issues. - For readonly classes, given their dependencies are likely to be readonly as well, they don't need to be deep cloned in most cases? - Therefore, I think combining __clone and clone-with will be a rare occasion, but all performance issues can be avoided by only using one of the two approaches in classes where that matters. - This RFC leaves future modifications to __clone open, so we could discuss a follow-up of passing the properties to __clone if we have a strategy to handle the BC issues, but for now, I see this as adding too much complexity for a very limited set of use cases that already are solved by the people that have them. So no situation will be worse than it was before, while some use-cases are enabled now and some performance critical code will probably stay the same as it is now. > - writing code with a wide range of supported PHP versions means we'll start using construct such as: > if (PHP_VERSION_ID>=3D80500) { > $b =3D 'clone'($a, [...]); // note 'clone' is a string here, so that th= e line is valid syntax on PHP <=3D 8.4 > else { > // another code path for PHP 8.4 > } Tim has already addressed the \namespace fallback and that this doesn't apply here, and that no performance penalty is introduced into any existing codebases. So my personal suggestion would to be for code that is doing custom things during cloning and support PHP pre-clone-with as is and only refactor once 8.5 is the minimum requirement, in places where it improves the code. Kind Regards, Volker --=20 Volker Dusch Head of Engineering Tideways GmbH K=C3=B6nigswinterer Str. 116 53227 Bonn https://tideways.io/imprint Sitz der Gesellschaft: Bonn Gesch=C3=A4ftsf=C3=BChrer: Benjamin Au=C3=9Fenhofer (geb. Eberlei) Registergericht: Amtsgericht Bonn, HRB 22127 --000000000000670a870636335521 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Nicolas,

Thank you for the great email and for thinking this through. Getting input= from the folks that maybe will use this feature the most is very valuable,= and I appreciate that you're taking=C2=A0the time to do this early.

On Mon, May 26, 2025 at 4:37= =E2=80=AFPM Nicolas Grekas <nicolas.grekas+php@gmail.com> wrote:
- To me, the main tech= nical drawback of the RFC is that it builds on mandatory deep-cloning, whic= h means wasted CPU/mem resources by design. This has to be mentioned in th= e RFC IMHO so that voters can make an informed decision.

About this last point, it's the one that makes me undecided abou= t my voting decision on the RFC.
I shared my concern and a soluti= on in another thread, where I also address the BC concern that is mentioned= in the RFC. Since this concern has been addressed, I think this should be = considered more closely. As is, that's a big omission to me - recognizi= ng and addressing the issue with deep-cloning.

Quoting from your other email:

> Allow to skip deep-cloning of already updated properties (that'= s a significant drawback of the current proposal - deep cloning before sett= ing is a potential perf/mem hog built into the engine) : guarding deep-clon= ing with a strict comparison would be enough.
=C2=A0
To= make sure we're on the same page: This would only come into effect if = both a __clone method exists and clone($thing, $withProperties) is used.

- We decided to __clone before updating properties t= o avoid BC issues.
- For readonly classes, given their dependenci= es are likely to be readonly as well, they don't need to be deep cloned= in most cases?
- Therefore, I think combining __clone and clone-= with will be a rare occasion, but all performance issues can be avoided by = only using one of the two approaches in classes where that matters.
- This RFC leaves future modifications=C2=A0to __clone open, so we could= discuss a follow-up of passing the properties to __clone if we have a stra= tegy to handle the BC issues, but for now, I see this as adding too much co= mplexity for a very limited set of use cases that=C2=A0already are solved b= y the people that have them.=C2=A0

So no sit= uation will be worse than it was before, while some use-cases are enabled n= ow and some performance critical code will probably=C2=A0stay the same as i= t is now.

> - writing code with a wide range of= supported PHP versions means we'll start using construct such as:
> if (PHP_VERSION_ID>=3D80500) {
>=C2=A0 =C2=A0$b = =3D 'clone'($a, [...]); // note 'clone' is a string here, s= o that the line is valid syntax on PHP <=3D 8.4
> else {
>=C2=A0 =C2=A0// another code path for PHP 8.4
> }<= /div>

Tim has already addressed the \namespace fallback = and that this doesn't apply here, and that no performance penalty is in= troduced into any existing codebases.

So my person= al suggestion would to be for code that is doing custom things during cloni= ng and support PHP pre-clone-with as is and only refactor once 8.5 is the m= inimum requirement,=C2=A0in places where it improves the code.=C2=A0
<= div>
Kind Regards,
Volker

--
--000000000000670a870636335521--