Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:123722 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 qa.php.net (Postfix) with ESMTPS id 3B8D41A009C for ; Fri, 21 Jun 2024 10:24:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1718965551; bh=yDMzb0tppxAXy30e+dmqudT8d3jHJqRAoBeYfuNohXI=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=RxLjMORw6TTsfZirDTR+ehPeRTALXnqpe3ebqm/xIQ4WlKJvIKgCW0QmDZR1wv7Yx ZEzgW1ig0KLiyxsTWd6L67zlqBlOsgJauXfUC+yqfp2JGocgrZXJ/2ZFp25saItKAF PdZkVo3xJrmTad6KFzDpu8wLkUvtGZCGYcl7kI1BkslKQBhJqc8jnpVX7kNhsYboqA lK/T+LaMQV+EzbJtLbFMYepmwgqBJkXuRW5GuFJhpJHTt9l5mVIsflkbBChKI+Fp3b SSA+l1yC0PVFKNCxIAje73lE2HxXBPW8/Decn/hn2yGDixo1ET6erRypILEyIhnScP wvGxfNyvOwsrw== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 4C82518006E for ; Fri, 21 Jun 2024 10:25:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) 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_PASS,FREEMAIL_FROM, HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: Error (Cannot connect to unix socket '/var/run/clamav/clamd.ctl': connect: Connection refused) X-Envelope-From: Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com [209.85.208.178]) (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 ; Fri, 21 Jun 2024 10:25:49 +0000 (UTC) Received: by mail-lj1-f178.google.com with SMTP id 38308e7fff4ca-2ec461d1005so14300141fa.0 for ; Fri, 21 Jun 2024 03:24:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718965474; x=1719570274; 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=h77EmXa9R7ztub2XKBpjUAEmfIk9yJ1Mp+5e5I2AF+E=; b=fZespzAjPY8uiSSMOa43h+cY2/inyo0DS8Wf2XyFU03s59fjBjUeIwvyB3tSvPsMCE netm1vCztc6B4I9j0NuDgwx6lutaeO7jCqoBb9fDx0MS7rXMSz51azzbDYzrs0Q2Azcs Vx57opVRe1B4+CwFdbI/NPpLgzYJ+B/662UxcMoubQz+V/R06tt4j+W1L9jEum3Q6hw+ SxtizlY6MOU1s3xnDtl2oY4Kt8ZIQamOGKjSuTBB0aBoQdQjIydGdThjGc0aZPCFche3 JN4w/gKTxXb9iATc2+uA+3RjxXZb3epSBJHKOp5oe6vpVdiMOrfnyd676OO1oH37ZFu+ YGHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718965474; x=1719570274; 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=h77EmXa9R7ztub2XKBpjUAEmfIk9yJ1Mp+5e5I2AF+E=; b=C+bEZkhN2R4z9SUbLetAsIXgN2ywo5+6OzdOCV7/5IzmiE5jMsBB4oqpOmRkm5ZqUw Kjsg09LA+eExbB2FGOi9XqkQQoChK9xGEFopg4roP1FoWi4cnxLguZ8lexUGtshLYTDr FLM1Gcjyk1JethBdRbD7/FYXqxBR3Bm9T/ZNY9iB8iMm4J53PXcgM7IXvRZkHe3rPtPc o09ic7zgHfv3U0+f2QoLJ7Ck8wfFmtFalmhFfknDRjJsx9jxvW4eC3/YP/lUMGCLmNtU agcXOICSn9tl3+kBo7PpVRCQRM3KVA6V9wo57Kk3M8Cf+oZKBaPZReN0Y3F0uS5ZgqBT AxMw== X-Forwarded-Encrypted: i=1; AJvYcCX9T/zkgCk9XKBnupxkXI+aJDCZU1KLNonYFvrbkhGN6UGgsD0REvwf5ITyU7wfqCWot6pEp/yOlzPfxLXlvMST16vQN9IegA== X-Gm-Message-State: AOJu0YxcwQUKTthVG6wD9IBHda36wIYOnHEbKN5ns3mpdrgAUiilMOOH YByTTf8zVMOmII97RIh4k3j6jiU+9N5TzYL01A+7TbNBvFyy8QBTXDCiXbFrC2+YKU95/KhVlDJ 6Aa6X0AlOWriz4uAy3b82fCqcpA2/RsRk8nA= X-Google-Smtp-Source: AGHT+IFd7VYCl1cFD0iN23oP30eDFBFZ8tWlNcdBUgKVf1XmvYVWjSY3joFYqA5h9WQ90SfDxLsxuhyDKRUSiC3QkfA= X-Received: by 2002:a19:6907:0:b0:52c:d703:d2fb with SMTP id 2adb3069b0e04-52cd703d3d3mr314434e87.17.1718965473561; Fri, 21 Jun 2024 03:24:33 -0700 (PDT) Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net MIME-Version: 1.0 References: <1118bbcd-a7b4-47bf-bf35-1a36ab4628e1@bastelstu.be> <8fc1969d-5f21-400d-a3ae-aee5d741a81d@app.fastmail.com> In-Reply-To: Date: Fri, 21 Jun 2024 12:24:20 +0200 Message-ID: Subject: Re: [PHP-DEV] [RFC] Lazy Objects To: =?UTF-8?Q?Benjamin_Au=C3=9Fenhofer?= Cc: Larry Garfield , php internals Content-Type: multipart/alternative; boundary="0000000000006ed335061b63d6e4" From: nicolas.grekas+php@gmail.com (Nicolas Grekas) --0000000000006ed335061b63d6e4 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ben, On Tue, Jun 18, 2024, at 5:45 PM, Arnaud Le Blanc wrote: >>> > Hi Larry, >>> > >>> > Following your feedback we propose to amend the API as follows: >>> > >>> > ``` >>> > class ReflectionClass >>> > { >>> > public function newLazyProxy(callable $factory, int $options): >>> object {} >>> > >>> > public function newLazyGhost(callable $initializer, int $options)= : >>> object {} >>> > >>> > public function resetAsLazyProxy(object $object, callable >>> > $factory, int $options): void {} >>> > >>> > public function resetAsLazyGhost(object $object, callable >>> > $initializer, int $options): void {} >>> > >>> > public function initialize(object $object): object {} >>> > >>> > public function isInitialized(object $object): bool {} >>> > >>> > // existing methods >>> > } >>> > >>> > class ReflectionProperty >>> > { >>> > public function setRawValueWithoutInitialization(object $object, >>> > mixed $value): void {} >>> > >>> > public function skipInitialization(object $object): void {} >>> > >>> > // existing methods >>> > } >>> > ``` >>> > >>> > Comments / rationale: >>> > - Adding methods on ReflectionClass instead of ReflectionObject is >>> > better from a performance point of view, as mentioned earlier >>> > - Keeping the word "Lazy" in method names is clearer, especially for >>> > "newLazyProxy" as a the "Proxy" pattern has many uses-cases that are >>> > not related to laziness. However we removed the word "Instance" to >>> > make the names shorter. >>> > - We have renamed "make" methods to "reset", following your feedback >>> > about the word "make". It should better convey the behavior of these >>> > methods, and clarify that it's modifying the object in-place as well >>> > as resetting its state >>> > - setRawValueWithoutInitialization() has the same behavior as >>> > setRawValue() (from the hooks RFC), except it doesn't trigger >>> > initialization >>> > - Renamed $initializer to $factory for proxy methods >>> > >>> > WDYT? >>> > >>> > Best Regards, >>> > Arnaud >>> >>> Oh, that looks *so* much more self-explanatory and readable. I love >>> it. Thanks! (Looks like the RFC text hasn't been updated yet.) >>> >> >> Happy you like it so much! The text of the RFC is now up to date. Note >> that we renamed ReflectionProperty::skipInitialization() and >> setRawValueWithoutInitialization() to skipLazyInitialization() and >> setRawValueWithoutLazyInitialization() after we realized that >> ReflectionProperty already has an isInitialized() method for something >> quite different. >> >> While Arnaud works on moving the code to the updated API, are there more >> comments on this RFC before we consider opening the vote? >> > > Thank you for updating the API, the RFC is now much easier to grasp. > > > My few comments on the updated RFC: > > > 1 ) ReflectionClass API is already very large, adding methods should use > naming carefully to make sure that users identify them as belonging to a > sub.feature (lazy objects) in particular, so i would prefer we rename som= e > of the new methods to: > > > isInitialized =3D> isLazyObject (with inverted logic) > > initialize =3D> one of initializeLazyObject / initializeWhenLazy / > lazyInitialize - other methods in this RFC are already very outspoken, so= I > don=E2=80=99t mind being very specific here as well. > > > The reason is =E2=80=9Einitialized=E2=80=9C is such a generic word, best = not have API > users make assumptions about what this relates to (readonly, lazy, =E2=80= =A6) > I get this aspect, I'm fine with either option, dunno if anyone has a strong preference? Under this argument, mine is isLazyObject + initializeLazyObject. 2.) I am 100% behind the implementation of lazy ghosts, its really great > work with all the behaviors. Speaking with my Doctrine ORM core developer > hat this has my full support. > \o/ > > 3.) the lazy proxies have me worried that we are opening up a can of > worms by having the two objects and the magic of using only the propertie= s > of one and the methods of the other. > > > Knowing Symfony DIC, the use case of a factory method for the proxy is a > compelling argument for having it, but it is a leaky abstraction solving > the identity issue only on one side, but the factory code might not know > its used for a proxy and make all sorts of decisions based on identity th= at > lead to problems. > > > Correct me if i am wrong or missing something, but If the factory does no= t > know about proxying, then it would also be fine to build a lazy ghost and > copy over all state after using the factory. > Unfortunately no, copying doesn't work in the generic case: when the object's dependencies involve a circular reference with the object itself, the copying strategy can lead to a sort of "brain split" situation where we have two objects (the proxy and the real object) which still coexist but can have diverging states. This is what virtual state proxies solve, by making sure that while we have two objects, we're sure by design that they have synchronized state. Yes, $this can leak with proxies, but this is reduced to the strict minimum in the state-proxy design. Compared to the "brain split" I mentioned, this is a minor concern. State-synchronization is costly currently since it relies on magic methods on every single property access. From this angle, state-proxies are the ones that benefit the most from being in the engine. 4.) I am wondering, do we need the resetAs* methods? You can already > implement lazy proxies in userland code by manually writing the code, we > don=E2=80=99t need engine support for that. Not having these two methods = would > reduce the surface of the RFC / API considerably. And given the =E2=80=9E= real > world=E2=80=9C example is not really real world, only the Doctrine > (createLazyGhost) and Symfony (createLazyGhost or createLazyProxy) are, > this shows maybe its not needed. > Yes, this use case of making an object lazy after it's been created is quite useful. It makes it straightforward to turn a class lazy using inheritance for example (LazyClass extends NonLazyClass), without having to write nor maintain any decorating logic. From a technical pov, this is just a different flavor of the same code infrastructure, so this is pretty aligned with the rest of the proposed API. > 5.) The RFC does not spell it out, but I assume this does not have any > effect on stacktraces, i.e. since properties are proxied, there are no > =E2=80=9Emagic=E2=80=9C frames appearing in the stacktraces? > Nothing special on this domain indeed, there are no added frames (unlike inheritance proxies since they'd decorate methods). As a general note, an important design criterion for the RFC has been to make it a superset of what we can achieve in userland already. Ghost objects, state proxies, capabilities of resetAsLazy* methods, etc are all possible today. Making the RFC a subset of those existing capabilities would defeat the purpose of this proposal, since it would mean we'd have to keep maintaining the existing code to support the use cases it enables, with all the associated drawbacks for the PHP community at large. Nicolas --0000000000006ed335061b63d6e4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Ben,

On Tue, Jun 18, 2024, at 5:45 PM, Arnaud Le Blanc wrote:
> Hi Larry,
>
> Following your feedback we propose to amend the API as follows:
>
> ```
> class ReflectionClass
> {
>=C2=A0 =C2=A0 =C2=A0public function newLazyProxy(callable $factory, int= $options): object {}
>
>=C2=A0 =C2=A0 =C2=A0public function newLazyGhost(callable $initializer,= int $options): object {}
>
>=C2=A0 =C2=A0 =C2=A0public function resetAsLazyProxy(object $object, ca= llable
> $factory, int $options): void {}
>
>=C2=A0 =C2=A0 =C2=A0public function resetAsLazyGhost(object $object, ca= llable
> $initializer, int $options): void {}
>
>=C2=A0 =C2=A0 =C2=A0public function initialize(object $object): object = {}
>
>=C2=A0 =C2=A0 =C2=A0public function isInitialized(object $object): bool= {}
>
>=C2=A0 =C2=A0 =C2=A0// existing methods
> }
>
> class ReflectionProperty
> {
>=C2=A0 =C2=A0 =C2=A0public function setRawValueWithoutInitialization(ob= ject $object,
> mixed $value): void {}
>
>=C2=A0 =C2=A0 =C2=A0public function skipInitialization(object $object):= void {}
>
>=C2=A0 =C2=A0 =C2=A0// existing methods
> }
> ```
>
> Comments / rationale:
> - Adding methods on ReflectionClass instead of ReflectionObject is
> better from a performance point of view, as mentioned earlier
> - Keeping the word "Lazy" in method names is clearer, especi= ally for
> "newLazyProxy" as a the "Proxy" pattern has many u= ses-cases that are
> not related to laziness. However we removed the word "Instance&qu= ot; to
> make the names shorter.
> - We have renamed "make" methods to "reset", follo= wing your feedback
> about the word "make". It should better convey the behavior = of these
> methods, and clarify that it's modifying the object in-place as we= ll
> as resetting its state
> - setRawValueWithoutInitialization() has the same behavior as
> setRawValue() (from the hooks RFC), except it doesn't trigger
> initialization
> - Renamed $initializer to $factory for proxy methods
>
> WDYT?
>
> Best Regards,
> Arnaud

Oh, that looks *so* much more self-explanatory and readable.=C2=A0 I love i= t.=C2=A0 Thanks!=C2=A0 (Looks like the RFC text hasn't been updated yet= .)

Happy you like it so much! The text = of the RFC is now up to date. Note that we renamed ReflectionProperty::skip= Initialization() and setRawValueWithoutInitialization() to skipLazyInitiali= zation() and setRawValueWithoutLazyInitialization() after we realized that = ReflectionProperty already has an isInitialized() method for something quit= e different.

While Arnaud works on moving the code= to the updated API, are there more comments on this RFC before we consider= opening the vote?

Thank you for updating the API, the RF= C is now much easier to grasp.


=

My few comments on the upda= ted RFC:


=

1 ) ReflectionClass API is = already very large, adding methods should use naming carefully to make sure= that users identify them as belonging to a sub.feature (lazy objects) in p= articular, so i would prefer we rename some of the new methods to:


=

=C2=A0isInitialized =3D>= isLazyObject (with inverted logic)

initialize =3D> one of i= nitializeLazyObject / initializeWhenLazy / lazyInitialize - other methods i= n this RFC are already very outspoken, so I don=E2=80=99t mind being very s= pecific here as well.


=

The reason is =E2=80=9Einit= ialized=E2=80=9C is such a generic word, best not have API users make assum= ptions about what this relates to (readonly, lazy, =E2=80=A6)


I get this aspect, I'm fine w= ith either option, dunno if anyone has a strong preference?
Under= this argument, mine is isLazyObject + initializeLazyObject.


2.) =C2=A0I am 100% behind = the implementation of lazy ghosts, its really great work with all the behav= iors. Speaking with my Doctrine ORM core developer hat this has my full sup= port.


\o/
<= div>=C2=A0


=

3.) =C2=A0the lazy proxies = have me worried that we are opening up a can of worms by having the two obj= ects and the magic of using only the properties of one and the methods of t= he other.=C2=A0


=

Knowing Symfony DIC, the us= e case of a factory method for the proxy is a compelling argument for havin= g it, but it is a leaky abstraction solving the identity issue only on one = side, but the factory code might not know its used for a proxy and make all= sorts of decisions based on identity that lead to problems.


=

Correct me if i am wrong or= missing something, but If the factory does not know about proxying, then i= t would also be fine to build a lazy ghost and copy over all state after us= ing the factory.


Unfortunately no, copying doesn't work in the generic case: when the = object's dependencies involve a circular reference with the object itse= lf, the copying strategy can lead to a sort of "brain split" situ= ation where we have two objects (the proxy and the real object) which still= coexist but can have diverging states.

This is wh= at virtual state proxies solve, by making sure that while we have two objec= ts, we're sure by design that they have synchronized state.
<= br>
Yes, $this can leak with proxies, but this is reduced to the = strict minimum in the state-proxy design. Compared to the "brain split= " I mentioned, this is a minor concern.

S= tate-synchronization is costly currently since it relies on magic methods o= n every single property access.
From this angle, state-proxie= s are the ones that benefit the most from being in the engine.


4.) =C2=A0I am wondering, d= o we need the resetAs* methods? You can already implement lazy proxies in u= serland code by manually writing the code, we don=E2=80=99t need engine sup= port for that. Not having these two methods would reduce the surface of the= RFC / API considerably. And given the =E2=80=9Ereal world=E2=80=9C example= is not really real world, only the Doctrine (createLazyGhost) and Symfony = (createLazyGhost or createLazyProxy) are, this shows maybe its not needed.<= /span>


Yes, this use case o= f making an object lazy after it's been created is quite useful. It mak= es it straightforward to turn a class lazy using inheritance for example (L= azyClass extends NonLazyClass), without having to write nor maintain any de= corating logic. From a technical pov, this is just a different flavor of th= e same code infrastructure, so this is pretty aligned with the rest of the = proposed API.
=C2=A0
5.) The RFC does = not spell it out, but I assume this does not have any effect on stacktraces= , i.e. since properties are proxied, there are no =E2=80=9Emagic=E2=80=9C f= rames appearing in the stacktraces?=C2=A0

Nothing special on this domain indeed, there are no= added frames (unlike inheritance proxies since they'd decorate methods= ).

As a general note, an important design criterio= n for the RFC has been to make it a superset of what we can achieve in user= land already. Ghost objects, state proxies, capabilities of resetAsLazy* me= thods, etc are all possible today. Making the RFC a subset of those existin= g capabilities would defeat the purpose of this proposal, since it would me= an we'd have to keep maintaining the existing code to support the use c= ases it enables, with all the associated drawbacks for the PHP community at= large.

Nicolas

--0000000000006ed335061b63d6e4--