Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123524
Message-ID: <1118bbcd-a7b4-47bf-bf35-1a36ab4628e1@bastelstu.be>
Date: Wed, 5 Jun 2024 20:25:17 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] [RFC] Lazy Objects
To: Arnaud Le Blanc <arnaud.lb@gmail.com>
Cc: Nicolas Grekas <nicolas.grekas+php@gmail.com>,
 PHP Internals List <internals@lists.php.net>
References: <CAOWwgpmbq5VRrZQvUXDsKiNK4r6+bFA4VxnjQ_U=h8T9r0o3DA@mail.gmail.com>
 <ab83af79-0669-47dd-a3cb-ab72327ae174@bastelstu.be>
 <CAP1Jc13JWVg99AULgzrGXcoCWJtJZGz+nr_VoyUocw8n5n5sfQ@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CAP1Jc13JWVg99AULgzrGXcoCWJtJZGz+nr_VoyUocw8n5n5sfQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=C3=BCsterhus?=)

Hi

Working through your reply in order of the email, without any 
backtracking, because the complexity of this topic makes it hard to keep 
the entire email in mind. This might mean that I am asking follow-up 
questions that you already answered further down. Please apologize if 
that is the case :-)

One general note: Please include the answers to my questions in the RFC 
text as appropriate for reference of other readers and so that all my 
questions are answered when re-reading the RFC without needing to refer 
back to your email.

On 6/5/24 16:50, Arnaud Le Blanc wrote:
>> Not a fan of flag parameters that take a bitset, those provide for a
>> terrible DX due to magic numbers. Perhaps make this a regular (named)
>> parameter, or an list of enum LazyObjectOptions { case
>> SkipInitOnSerialize; }?
> 
> The primary reason for choosing to represent $options as a bitset is
> that it's consistent with the rest of the Reflection API (e.g.
> ReflectionClass::getProperties() uses a bitset for the $filter
> parameter).
> 
> I don't get your point about magic numbers since we are using
> constants to abstract them.

It's not a magic number in the classic sense, but when trying to observe 
it, e.g. by means of a debugger and the $options have been passed 
through some other functions that wrap the lazy object API, it will 
effectively be an opaque number that one will need to decode manually, 
whereas a list of enums is immediately clear.

>> My understanding is that the $object that is passed to the first
>> parameter of makeLazyProxy() is completely replaced. Is this
>> understanding correct? What does that mean for spl_object_hash(),
>> spl_object_id()? What does this mean for WeakMap and WeakReference? What
>> does this mean for objects that are only referenced from within $object?
> 
> The object is updated in-place, and retains its identity. It is not
> replaced. What makeLazyGhost() and makeLazyProxy() do is equivalent to
> calling `unset()` on all properties, and setting a flag on the object
> internally. Apart from setting the internal flag, this is achievable

Oh. It was not clear at all to me that all existing properties will be 
unset. Did I miss it or is that not written down in the RFC?

Is there any reason to call the makeLazyX() methods on an object that 
was not just freshly created with ->newInstanceWithoutConstructor() 
then? Anything I do with the object before the call to makeLazyX() will 
effectively be reverted, no?

An example showcasing the intended usage, e.g. a simplified ORM example, 
would really be helpful here.

> in userland by iterating on all properties via the Reflection API, and
> using unset() in the right scope with a Closure.
> 
> spl_object_id(), spl_object_hash(), SplObjectStorage, WeakMap,
> WeakReference, strict equality, etc are not affected by makeLazy*().

That is true for *both* makeLazyGhost(), and makeLazyProxy()?

What would the following example output?

     $object = new MyObject();
     var_dump(spl_object_id($object));
     $r = ReflectionLazyObject::makeLazyGhost($object, function 
(MyObject $object) {
         $object2 = new MyObject();
         var_dump(spl_object_id($object2));
         return $object2;
     });
     var_dump(spl_object_id($object));
     $r->initialize();
     var_dump(spl_object_id($object));

What would happen if I would expose the inner $object2 to the outer 
world by means of the super globals or by means of `use (&$out)` + `$out 
= $object2`?

> The intended use of makeLazyGhost() and makeLazyProxy() is to call
> them either on an object created with
> ReflectionClass::newInstanceWithoutConstructor(), or on $this in a
> constructor. The latter is the reason why these APIs take an existing
> object.

Okay, that answers the question above. Technically being capable of 
calling it on an object that was not just freshly created sounds like a 
footgun, though. What is the interaction with readonly objects? My 
understanding is that it would allow an readonly object with initialized 
properties to change after-the-fact?

>> Consider this example:
>>
>>       class Foo {
>>         public function __destruct() { echo __METHOD__; }
>>       }
>>
>>       class Bar {
>>         public string $s;
>>         public ?Foo $foo;
>>
>>        public function __destruct() { echo __METHOD__; }
>>       }
>>
>>       $bar = new Bar();
>>       $bar->foo = new Foo();
>>
>>       ReflectionLazyObject::makeLazyProxy($bar, function (Bar $bar) {
>>         $result = new Bar();
>>         $result->foo = null;
>>         $result->s = 'init';
>>         return $result;
>>       });
>>
>>       var_dump($bar->s);
>>
>> My understanding is that this will dump `string(4) "init"`. Will the
>> destructor of Foo be called? Will the destructor of Bar be called?
> 
> This will print:
> 
>      Foo::__destruct (during makeLazyProxy())
>      string(4) "init" (during var_dump())
> 
> and eventually
> 
>      Bar::__destruct (when $bar is released)

Okay, so only one Bar::__destruct(), despite two Bar objects being 
created. I assume it's the destructor of the second Bar, i.e. if I would 
dump `$this->foo` within the destructor, it would dump `null`?

>> - What happens if I make an object lazy that already has all properties
>> initialized? Will that be a noop? Will that throw? Will that create a
>> lazy object that will never automatically be initialized?
> 
> All properties are unset as described earlier, and the object is
> flagged as lazy. The object will automatically initialize when trying
> to observe its properties.
> 
> However, making a fully initialized object lazy is not the intended use-case.

Understood. See above with my follow-up question then.

>> - Before calling the initializer, properties that were not initialized
>> with ReflectionLazyObject::skipProperty(),
>> ReflectionLazyObject::setProperty(),
>> ReflectionLazyObject::setRawProperty() are initialized to their default
>> value.
>>
>> Should skipProperty() also skip the initialization to the default value?
>> My understanding is that it allows skipping the initialization on
>> access, but when initialization actually happens it should probably be
>> set to a well-defined value, no?
>>
>> Am I also correct in my understanding that this should read "initialized
>> to their default value (if any)", meaning that properties without a
>> default value are left uninitialized?
> 
> The primary effect of skipProperty() is to mark a property as
> non-lazy, so that accessing it does not trigger the initialization of
> the entire object. It also sets the property to its default value if
> any, otherwise it is left as undef.
> 
> Accessing this property afterwards has exactly the same effect as
> doing so on an object created with
> ReflectionClass::newInstanceWithoutConstructor() (including triggering
> errors when reading an uninitialized property).

I'm rereading my own question and can't make sense of it any more. I 
probably forgot that skipProperty() is defined to set the default value 
in the PHPDoc when I got down to the bit that I quoted.

Please just insert the 'if any' after 'default value' for clarity.

>> Will the "revert to its pre-initialization"
>> work properly when you have nested lazy objects? An example would
>> probably help.
> 
> Only the effects on the object itself are reverted. External side
> effects are not reverted.

Yes, it's obvious that external side effects are not reverted. I was 
thinking about a situation like:

     $a = new A();
     $b = new B();
     ReflectionLazyObject::makeLazyGhost($b, function ($b) {
         throw new \Exception('xxx');
     });
     ReflectionLazyObject::makeLazyGhost($a, function ($a) use ($b) {
         $a->b = $b->somevalue;
     });
     $a->init = 'please';

The initialization of $a will implicitly attempt to initialize $b, which 
will fail. Am I correct in my understanding that both $a and $b will be 
reverted back to a lazy object afterwards? If so, adding that example to 
the RFC would help to make possible edge cases clear.

>> - The return value of the initializer has to be an instance of a parent
>> or a child class of the lazy-object and it must have the same properties.
>>
>> Would returning a parent class not violate the LSP? Consider the
>> following example:
>>
>>       class A { public string $s; }
>>       class B extends A { public function foo() { } }
>>
>>       $o = new B();
>>       ReflectionLazyObject::makeLazyProxy($o, function (B $o) {
>>         return new A();
>>       });
>>
>>       $o->foo(); // works
>>       $o->s = 'init';
>>       $o->foo(); // breaks
> 
> $o->foo() calls B::foo() in both cases here, as $o is always the proxy
> object. We need to double check, but we believe that this rule doesn't
> break LSP.

I don't understand what happens with the 'A' object then, but perhaps 
this will become clearer once you add the requested examples.

Best regards
Tim Düsterhus