Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:123717 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 CBF151A009C for ; Fri, 21 Jun 2024 00:38:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1718930367; bh=XU0ZRHTR/5aOs+DViDz3KlubPOUdXHqKOhlQGY5QODE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=lN6vDM1wTlQ0YvahQR78ixFJ3+oegCvooajHxYL9Emce1NZ9NpWD9QlgqzoL2d6Kz Oo9Ks64TmmB7NBmJPIz8qZe0V7DFLNuziV7G9e0I9BAiCvyoOR7rgBR+RZu8feBYIL Vv7GTwd0kFoXp1yJXEHSF2YHOswySMSVLZ+OFz8eOValxhWYR//MeW9e4634/bgAPJ szekveyWt+P2gOF/DN1s028VYYwKsKqqEf39SR4oQNxWgmtaE46lut1lJ6aBoveu+I yYRfhC9pHaPV0DhCUIe86BJffCUorX6P30CFlfZ8jQRvXjule4bVeJjOod+7n/4qMS H7jIenr4GQo/A== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id BB01318005A for ; Fri, 21 Jun 2024 00:39:25 +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,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-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) (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 00:39:25 +0000 (UTC) Received: by mail-vs1-f46.google.com with SMTP id ada2fe7eead31-48d9cbfe0ffso463116137.2 for ; Thu, 20 Jun 2024 17:38:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=beberlei-de.20230601.gappssmtp.com; s=20230601; t=1718930290; x=1719535090; 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=xQx3mgLNgMj4KFEMLmIJfvhjrLd0YdIYEXJ1q8xbccQ=; b=iz2oPTkrIDsjq+tEYLf8XNXm11x8Gb2ohqAHhwjSuSb06wuJC9JlLTdeqglsFz5QDh 1f9oMR4g1Pv+4FvqAaNnHmggJyq5bmLfqPfrj30OOoVafIaYSujLWTLCANHUAEIFHA1B ItMfuH5dgvn1TAyvyOHHsPjY7v1iRLISak/FJNBny16IyitYvENQh1vSpWnpk6ln7GLh EFc7v/ivI8elNE+vyx4GAdSTKBxXR7WCMwlqQ9PmaT16bkCzI4hMfuaJdtVEE2aFDtyu lKA9BMC3RXupI0fcZMZB08vFBJDp/Mkz6liTnMon6icqegpr5JAy2mO8ar8Cyg1Hybo7 rcsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718930290; x=1719535090; 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=xQx3mgLNgMj4KFEMLmIJfvhjrLd0YdIYEXJ1q8xbccQ=; b=hPKv0o6hJ+eblz/9N3+Lrl/N0AHfCY9bYqirQ3hOSOI9NDHE2/txBNFUb4oD4eVGEu SntsygXOQqsBHPZBz8uRD661gIqZzXnW8lhlrkTf8bBUf6SGWYOW/w6fDTx5GInINlET ONfYeOc1KAJ8DpllHPk6eLq1fQhgM4dwp23AUHqEGveN7RUFB4m/GDC0WfvZ4TjznEva X9dEsbAAq9hLfYkxjrZEL7FOLtG6BAdavpVsfxjrxB5wKthY+M+CrLeNGDaJokc9lyge DlnKxFD2Svi4UVsJj2B+W54HRSu2eOX8nVpT9f8NFovErLpN2r/iNVm+yiyiD13R8ZEa zq2A== X-Forwarded-Encrypted: i=1; AJvYcCWuDEykDLUhwFlNkKqPkA4U6fQII6/DeXE9lg2PzoX5uBc95/usRxYLNU9BiItmct29DWotpT93GJIaMtcweHMrAdYxX/6NpA== X-Gm-Message-State: AOJu0YxERtC9xhCJF/FR2CzV3aRMwsqayQWWCpIlTQehhuBx5zEVJYAq 4eog3xmpVBW+gr8GpbHRpfsHAYFxQ4tkE9HmIB6xMKe+TPxaiVZ4T+nNcz+p0tAYw1IrTDUUieO p9HQ0LftjbkyNQZiEsY0FrDIJuq+73AO/RjEjv0RdGHotW85bvP8= X-Google-Smtp-Source: AGHT+IHPrWByy1LQlGsYjn6SE/pS7KUhrMKp69Gk3RGOgSiCHoohD+1RtSFTVVJq1+e8gK84w2KtM6UsTsAt92ZbOBI= X-Received: by 2002:a67:fc44:0:b0:48c:3746:33e2 with SMTP id ada2fe7eead31-48f1303bc85mr7108205137.14.1718930290166; Thu, 20 Jun 2024 17:38:10 -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 02:37:58 +0200 Message-ID: Subject: Re: [PHP-DEV] [RFC] Lazy Objects To: Nicolas Grekas Cc: Larry Garfield , php internals Content-Type: multipart/alternative; boundary="00000000000056ea22061b5ba578" From: kontakt@beberlei.de (=?UTF-8?Q?Benjamin_Au=C3=9Fenhofer?=) --00000000000056ea22061b5ba578 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Jun 20, 2024 at 10:52=E2=80=AFAM Nicolas Grekas < nicolas.grekas+php@gmail.com> wrote: > > > Le mar. 18 juin 2024 =C3=A0 22:59, Larry Garfield a > =C3=A9crit : > >> 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 some 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 no= t have API users make assumptions about what this relates to (readonly, lazy, =E2=80=A6) 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. 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 properties 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 that 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 ghost and copy over all state after using the factory. This creates a similar amount of problems with identity, but is less magic while doing so. All of the inheritance heirachy and properties must exist logic can also be implemented in the userland initializer, passing the responsibility for the mess over to userland ;) class Container { public function getClientService(): Client { $reflector =3D new ReflectionClass(Client::class); $client =3D $reflector->newLazyGhost(function (Client $ghost) use ($container) { $clientFactory =3D $container->get('client_factory'); $client =3D $clientFactory->createClient(); // not sure this is 100% right, the idea is to copy all state over $vars =3D get_mangled_object_vars($client); foreach ($vars as $k =3D> $v) { $ghost->$k =3D $v; } }); return $client; } This would also allow to make =E2=80=9Einitialize=E2=80=9C return void and = simplify this part of the API. 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 wo= uld reduce the surface of the RFC / API considerably. And given the =E2=80=9Ere= al 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. 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? > > Cheers, > Nicolas > --00000000000056ea22061b5ba578 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Jun 20, 2024 at 10:52=E2=80= =AFAM Nicolas Grekas <= nicolas.grekas+php@gmail.com> wrote:


Le=C2=A0mar. 18 juin 2024 =C3=A0=C2=A022:5= 9, Larry Garfield <larry@garfieldtech.com> a =C3=A9crit=C2=A0:
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)


=

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.


=

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. This creates a similar amount of problems with identity, b= ut is less magic while doing so. All of the inheritance heirachy and proper= ties must exist logic can also be implemented in the userland initializer, = passing the responsibility for the mess over to userland ;)


=

class Container

{

=C2=A0 =C2=A0 public function getClientService(): C= lient

=C2=A0 =C2=A0 {

=C2=A0 =C2=A0 =C2=A0 =C2=A0 $reflector =3D new Refl= ectionClass(Client::class);

=C2=A0

=C2=A0 =C2=A0 =C2=A0 =C2=A0 $client =3D $reflector-= >newLazyGhost(function (Client $ghost) use ($container) {

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $clientFa= ctory =3D $container->get('client_factory');

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $client = =3D $clientFactory->createClient();


=

=C2=A0 =C2=A0 // not sure this is 100% right, the idea = is to copy all state over

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $vars =3D= get_mangled_object_vars($client);

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 foreach (= $vars as $k =3D> $v) { $ghost->$k =3D $v; }

=C2=A0 =C2=A0 =C2=A0 =C2=A0 });

=C2=A0

=C2=A0 =C2=A0 =C2=A0 =C2=A0 return $client;<= /p>

=C2=A0 =C2=A0 }


=

This would also allow to ma= ke =E2=80=9Einitialize=E2=80=9C return void and simplify this part of the A= PI.


=

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>


=

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

Cheers,
Nicolas
--00000000000056ea22061b5ba578--