Am 21.06.2024, 12:24:20 schrieb Nicolas Grekas <nicolas.grekas+...@gmail.com
>:
> 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.
>
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 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.
>
>
> 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.
>
>
>> 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.
>
> Nicolas
>
>
