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 PM Nicolas Grekas <nicolas.grekas+...@gmail.com>
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 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 - recognizing 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 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>=80500) {
> $b = 'clone'($a, [...]); // note 'clone' is a string here, so that the
line is valid syntax on PHP <= 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
--
Volker Dusch
Head of Engineering
Tideways GmbH
Königswinterer Str. 116
53227 Bonn
https://tideways.io/imprint
Sitz der Gesellschaft: Bonn
Geschäftsführer: Benjamin Außenhofer (geb. Eberlei)
Registergericht: Amtsgericht Bonn, HRB 22127
