On Fri, Jun 21, 2024 at 12:24 PM Nicolas Grekas <
nicolas.grekas+...@gmail.com> wrote:
> 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 some
>> of the new methods to:
>>
>>
>> isInitialized => isLazyObject (with inverted logic)
>>
>> initialize => one of initializeLazyObject / initializeWhenLazy /
>> lazyInitialize - other methods in this RFC are already very outspoken, so I
>> don’t mind being very specific here as well.
>>
>>
>> The reason is „initialized“ is such a generic word, best not have API
>> users make assumptions about what this relates to (readonly, lazy, …)
>>
>
> 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 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.
>>
>
> 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.
>
Makes sense to me.
>
>
> 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’t need engine support for that. Not having these two methods would
>> reduce the surface of the RFC / API considerably. And given the „real
>> world“ 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.
>
Will you use this in Symfony DIC for something? While I can understand the
argument that its easy to integrate with the lazy object code that you
have, i don't see how the argument "without having to write nor maintain
any dceoarting logic" is true.
The example in the RFC is very much written code. Yes its only one line of
new ReflectionClass()->initialie($this)->send($data), but something like
https://gist.github.com/beberlei/568719a1c5536cc5f59a60381c37aa05 is not
more code and works fine.
The (Lazy-)Connection example can be re-written as:
class LazyConnection extends Connection
{
public function create(): Connection
{
return (new
ReflectionClass(Connection::class))->newLazyGhost(function (Connection
$connection) {
$connection->__construct(); // Or any heavier initialization
logic
$connection->ttl = 2.0;
});
}
private function __construct() {
parent::__construct();
}
}
This to me reads easier, especially when Connection has more than one
public method (send) it requires way less code.
Given the complexities of newLazy* already, i am just trying to find
arguments to keep the public surface of this API as small as posisble, as
its intricacies are hard to grasp and simplicity / less ways to use it will
be a benefit.
So far i don't see that with resetAsLazy* you can impmlement something new
that cannot also be done with newLazy* methods.
>
>
>> 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
>> „magic“ 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.
>
I very much appreciate the benefits this brings as primary language concept.
>
> Nicolas
>
>
