Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:124406 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 1C8AA1A00B7 for ; Fri, 12 Jul 2024 10:55:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1720781823; bh=DbGIFXBINB4jh4lR5f/vW873lvwkXP7jUm7L3sYC4EE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Q9ypRpF8iaPyisP4r9Za/supB/qIXs67DoQQ82bQWCnVoFBWQtmeppyEmOSeMK5x+ xUaARnzvygd5YLlk+bRel6pI19HugYjw9zHbAs55R3+kUUp7mowwivMPVA2kcu3pz1 0Ulu/QgDDOk100lhsptgQ60rbWJybwEI3jaIJ0b59nbFt/DKGHdAAATKfiXllHFcbj wHvd8Ji9GOrMoweU8Z7eQtiPQBrBoCwzMsLeBgiDJbt3HY0OxMpIr/uk7GSQ2w0djr TC+Lswsog2Mxgk7S3H5qZALZlba6ve5ph2yAG5x779XNoqrkSpdYCCsaQs/QSXtKvN ClX5jZ3IKi5Fw== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id C275818007E for ; Fri, 12 Jul 2024 10:57:02 +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.8 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DMARC_MISSING,HTML_MESSAGE,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) (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, 12 Jul 2024 10:57:02 +0000 (UTC) Received: by mail-vs1-f50.google.com with SMTP id ada2fe7eead31-48ffdfae096so380663137.0 for ; Fri, 12 Jul 2024 03:55:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=beberlei-de.20230601.gappssmtp.com; s=20230601; t=1720781734; x=1721386534; 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=V12aptpoNc8/TVmp50k85pWP3Y7Ucu3ewqJbP/ajMC8=; b=rRrO56zzvLnMfjEgiiYdJum+bQHLudqGMVcl8amEwl8hQNFD1q3wm7gOpFcPGqgr7O dzDV3jC0dIuHR+ZUEuO/ubacH637XfWo5USj6Yi0hGURgh1pUPGkIu3bpMi9rJOPAerP ELLOuwxC64F+ePn6ZFil4LO69hTckm+8GIBPUuWEJyN6SWBrFBxMQbxq9oPpiU8PtgMM MsD9BD5PLtVxFhi9WCumWVvk+vvzIXfM6UD4X7ctdkI4ruTMe3/7ZdOIPphs3nT94wbx 1I9L8GXM/VP/mkCL5U8LpgiylCJFqmk6+9uiGCO2AIGBM9J7N4lLAsNA4Eu78ytyJg1A 2OHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720781734; x=1721386534; 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=V12aptpoNc8/TVmp50k85pWP3Y7Ucu3ewqJbP/ajMC8=; b=RrD5Wfl8GkZkX2hRUAsRowjZYJp0VTLoLe4L77iVDqRI5jmxK3xLa/6snpbIrwaJma WIrNbOEiavBWV8jR/Bomp04wq+NoUao3VHSITtUAq58NG7xwWCbsnA65GUuFw5wNHQpC w7Kefv/E9nIan7j/0sQv1MFAzRtqyAGbqEkuWx+NNpwLIntScerDsOWAxnWLDVGZ9f/h GLCNMgBS5UygpxVf7SB32Ej9JLIKV/wEO9aSCyqxUfD7v0+WijYQ9cnjYt6JDjBxCB22 PGY2X+7umzBxNUrAOwfAvhOgA1vWc1V2qnYxf1B1r5zPvkX8bShO0j7q89VQ01k81Lk1 hxig== X-Forwarded-Encrypted: i=1; AJvYcCXjMQuaYiOXNNUPEAOW6F4ouWq7HGRp5BI5/8XXHmjqZjRRhAx62+6l4WUjRO2IUxhqv2gD+LCqFOhuJnlpXbCbCn9RCu6oCw== X-Gm-Message-State: AOJu0YxEAxqG2dfmcZ3uFm23ch9w66odJola25jlbtywu73ykIk1Mpw2 0O2DS+maYhXCGD0YisNMvPPaxZg9fWrIFdRcbhCUFQB4awR7x7qWURfTq9GBY9ktjx1VUhDxByD H2G5kn3eOrOPW9VHLtFBmjuEzsRRteNF1pO8KaQ== X-Google-Smtp-Source: AGHT+IE2lM0U22faQB3HJlry68LSrxyXiX6HIWYcKP+rUUnBQKgS8zKko6vspk0wudMS+2biaYyIA8+mMzjWF/0bSzA= X-Received: by 2002:a05:6102:b12:b0:48f:2404:ea7d with SMTP id ada2fe7eead31-4903216da57mr12742354137.13.1720781733972; Fri, 12 Jul 2024 03:55:33 -0700 (PDT) Received: from 1064022179695 named unknown by gmailapi.google.com with HTTPREST; Fri, 12 Jul 2024 12:55:32 +0200 Received: from 1064022179695 named unknown by gmailapi.google.com with HTTPREST; Fri, 12 Jul 2024 12:55:28 +0200 Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net MIME-Version: 1.0 (Mimestream 1.3.6) References: <1118bbcd-a7b4-47bf-bf35-1a36ab4628e1@bastelstu.be> <8fc1969d-5f21-400d-a3ae-aee5d741a81d@app.fastmail.com> In-Reply-To: Date: Fri, 12 Jul 2024 12:55:32 +0200 Message-ID: Subject: Re: [PHP-DEV] [RFC] Lazy Objects To: Nicolas Grekas Cc: Larry Garfield , php internals Content-Type: multipart/alternative; boundary="000000000000fd6adc061d0ab7a3" From: kontakt@beberlei.de (=?UTF-8?Q?Benjamin_Au=C3=9Fenhofer?=) --000000000000fd6adc061d0ab7a3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Am 21.06.2024, 12:24:20 schrieb Nicolas Grekas : > 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 mor= e >>> 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 so= me >> 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, s= o 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. > The RFC still has the isInitialized and initialize methods, lets go with your suggestions isLazyObject, initializeLazyObject, and also maybe markLazyObjectAsInitialized instead of markAsInitialized? > > > 2.) I am 100% behind the implementation of lazy ghosts, its really great >> work with all the behaviors. Speaking with my Doctrine ORM core develope= r >> 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 properti= es >> 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 t= hat >> lead to problems. >> >> >> Correct me if i am wrong or missing something, but If the factory does >> not know about proxying, then it would also be fine to build a lazy ghos= t >> 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 method= s > 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= =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. >> > > 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 ju= st > 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 > > --000000000000fd6adc061d0ab7a3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Am 21.06.2024, 12:24:20 schrieb N= icolas Grekas <nicolas= .grekas+php@gmail.com>:
Hi Ben,

On Tue, Jun 18, 2024, at 5:45 PM, Arnaud Le Blanc wr= ote:
> 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.

The RFC still has the isInitialized and = initialize methods, lets go with your suggestions isLazyObject, initializeL= azyObject, and also maybe markLazyObjectAsInitialized instead of markAsInit= ialized?


=

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

--000000000000fd6adc061d0ab7a3--