RFC Weakrefs

Benjamin Morel

7 years ago

I was precisely hoping there was a WeakRef implementation the other day.
This would be very useful for a simple data mapper, where you could keep
references to loaded objects and a cache of their original properties, to
compute the changeset to sync to the DB when save()ing them.
WeakRefs would allow to automatically clean up the cache of loaded
properties when the object goes out of scope and is reclaimed by the GC.

So, FWIW, big +1 from me, and thank you for the RFC and the PR! 👍

Ben

On Sat, 2 Feb 2019 at 09:35, Joe Watkins <krakjoe@php.net> wrote:

Christoph Becker

7 years ago

On 02.02.2019 at 09:35, Joe Watkins wrote:

> Some time ago I brought this up for discussion, and last night was reminded
> of it's existence, and so this morning rebased and reworked the patch a
> little based on the feedback I got back then.
>
> Since it was long ago, and there's no particular rush, I don't intend to
> open voting until the current policy adjustment RFC's are resolved, but
> wanted to give everyone a heads up and ask for any feedback you may have at
> this time.
>
> https://wiki.php.net/rfc/weakrefs

Overall I like this.  Thanks!

However, calling this class WeakRef would introduce a potential BC
break, and would be a particular issue regarding the PHP manual, which
does not allow to document two classes with exactly the same name.

Even if Etienne would officially cease the further development of
PECL/Weakref[1], having a class with the same name appears to be
confusing at best.

Maybe another name should be chosen for the class?  Or maybe we should
finally introduce the \PHP namespace?

[1] <https://pecl.php.net/package/Weakref>

--
Christoph M. Becker

Joe Watkins

7 years ago

Afternoon Christoph,

I can't really think of another name ... it's ... a weakref ...

Probably using the PHP namespace makes most sense.

Cheers
Joe

On Sun, 3 Feb 2019 at 18:40, Christoph M. Becker <cmbecker69@gmx.de> wrote:

Robert Korulczyk

7 years ago

> I can't really think of another name ... it's ... a weakref ...

It is actually "weak reference", so why not WeakReference?


Regards,
Robert Korulczyk

Joe Watkins

7 years ago

Does that solve the problem for you Christoph ?

Cheers
Joe

On Sun, 3 Feb 2019 at 19:08, Robert Korulczyk <robert@korulczyk.pl> wrote:

Joe Watkins

7 years ago

Updated patch, name is WeakReference.

Cheers
Joe

On Sun, 3 Feb 2019 at 19:39, Joe Watkins <krakjoe@gmail.com> wrote:

Christoph Becker

7 years ago

On 03.02.2019 at 19:39, Joe Watkins wrote:

> Does that solve the problem for you Christoph ?

Yes, calling the class WeakReference would be fine for me.

--
Christoph M. Becker

Joe Watkins

7 years ago

Already taken care of :)

On a side note, at what point do we remove stuff from the manual/pecl, the
weakref is extension is dead, can't work with current versions of PHP, and
there was never a stable release ?

I'm not even sure we have a mechanism to delete stuff from pecl ... there's
probably quite a lot of junk on there ...

Cheers
Joe

On Sun, 3 Feb 2019 at 19:56, Christoph M. Becker <cmbecker69@gmx.de> wrote:

Peter Kokot

7 years ago

Hello,

On Sun, 3 Feb 2019 at 20:00, Joe Watkins <krakjoe@gmail.com> wrote:

>
> Already taken care of :)
>
> On a side note, at what point do we remove stuff from the manual/pecl, the
> weakref is extension is dead, can't work with current versions of PHP, and
> there was never a stable release ?
>
> I'm not even sure we have a mechanism to delete stuff from pecl ... there's
> probably quite a lot of junk on there ...
>

Since PECL extensions is one whole compilation of extensions working
"together" and not falling into conflicts with names and similar
issues, PECL admins can simply set the package as "superseded by"
another package. For example:
https://pecl.php.net/package/date_time

If another "weakref" PHP extension will be developed elsewhere,
different naming can be picked for PECL or if that extension will
return to development, different approaches should be done probably
for it...

--
Peter Kokot

Christoph Becker

7 years ago

On 03.02.2019 at 20:05, Peter Kokot wrote:

> On Sun, 3 Feb 2019 at 20:00, Joe Watkins <krakjoe@gmail.com> wrote:
>>
>> On a side note, at what point do we remove stuff from the manual/pecl, the
>> weakref is extension is dead, can't work with current versions of PHP, and
>> there was never a stable release ?
>>
>> I'm not even sure we have a mechanism to delete stuff from pecl ... there's
>> probably quite a lot of junk on there ...
>
> Since PECL extensions is one whole compilation of extensions working
> "together" and not falling into conflicts with names and similar
> issues, PECL admins can simply set the package as "superseded by"
> another package. For example:
> https://pecl.php.net/package/date_time
>
> If another "weakref" PHP extension will be developed elsewhere,
> different naming can be picked for PECL or if that extension will
> return to development, different approaches should be done probably
> for it...

Not sure about removal from the PHP manual.  This may never have
happened before (except for PECL/idn which conflicted with ext/intl).
Might be better to discuss this on the doc mailing list.

F'up2 <phpdoc@lists.php.net>

--
Christoph M. Becker

Michael Wallner

7 years ago

On 03/02/2019 22:49, Christoph M. Becker wrote:

>
> Not sure about removal from the PHP manual.  This may never have
> happened before (except for PECL/idn which conflicted with ext/intl).
> Might be better to discuss this on the doc mailing list.
>
> F'up2 <phpdoc@lists.php.net>
>

Oh, it happened. I deleted http-v1 docs a few years ago.

--
Regards,
Mike

Andrew Faulds

7 years ago

Hello,

Joe Watkins wrote:

> Already taken care of :)
>
> On a side note, at what point do we remove stuff from the manual/pecl, the
> weakref is extension is dead, can't work with current versions of PHP, and
> there was never a stable release ?
>
> I'm not even sure we have a mechanism to delete stuff from pecl ... there's
> probably quite a lot of junk on there ...
>
> Cheers
> Joe

The situation with PECL in the PHP manual has bothered me for a while. I
don't know (maybe I could find out) the reasons for mixing PECL and
bundled extensions together in one manual, but considering the confusion
it can creates and the undue prominence it gives (obscure
poorly-maintained PECL extensions get the same accessibility as strlen,
which can make PHP look bad as it seems like stuff like runkit is part
of the language…) I would be in favour of splitting the PECL extensions
into their own manual, or doing what some other languages do and just
let each extension have its own documentation to itself.

Thanks,
Andrea

Andrew Faulds

7 years ago

Hi Joe,

I like the idea of adding weak references to the PHP core (as opposed to
PECL), so I'm generally in favour of this RFC. This type of feature
which the engine must be aware of should be maintained as part of it —
and it not requiring a PECL extension means it can actually be used by
much more code.

Have you considered adding a WeakMap type as well — a map of (object)
keys to values where the keys are weakly referenced? This is a useful
construct based on weak references and something I'm sure will be
wanted. It could in theory be implemented in userland on top of your
proposal, but it would be nice to save userland having to reimplement it
(probably multiple times), and userland could be saved the problem of
manually cleaning up now-dead weak references (annoyingly O(n)) and
deciding when to do so — an internal implementation can handle it
automagically and more performantly. :)

I do predict one potentially annoying part of implementing a WeakMap
though: you could insert a value which has a destructor and then observe
when keys are destructed. But it's not like destructors are a new
problem in PHP.

Thanks,
Andrea

Dennis Birkholz

7 years ago

Hi Joe,

On 02.02.19 09:35, Joe Watkins wrote:

> Some time ago I brought this up for discussion, and last night was reminded
> of it's existence, and so this morning rebased and reworked the patch a
> little based on the feedback I got back then.
>
> Since it was long ago, and there's no particular rush, I don't intend to
> open voting until the current policy adjustment RFC's are resolved, but
> wanted to give everyone a heads up and ask for any feedback you may have at
> this time.

thanks for proposing to bring weak references into core, I myself would
have needed them once or twice.

One point from the RFC bothers me:

> The proposed API:
> - does not support serialization

That is a really unfortunate decision, I would really like to have them
serializable. Otherwise if you want to persist a weak ref you always
have to convert them to a real ref before serialization and back
afterwards. I would really like to have this transparency build in.
Would be great if that could be added.

I am a little surprised how often the "is not serializable" label is put
on something proposed in an RFC and how little that seem to bother
anybody. That just makes serializing data much more complicated/prone to
errors/exception.

Thanks for your work,
Dennis

Nikita Popov

7 years ago

On Tue, Feb 5, 2019 at 4:11 PM Dennis Birkholz <php@dennis.birkholz.biz>
wrote:

> Hi Joe,
>
> On 02.02.19 09:35, Joe Watkins wrote:
> > Some time ago I brought this up for discussion, and last night was
> reminded
> > of it's existence, and so this morning rebased and reworked the patch a
> > little based on the feedback I got back then.
> >
> > Since it was long ago, and there's no particular rush, I don't intend to
> > open voting until the current policy adjustment RFC's are resolved, but
> > wanted to give everyone a heads up and ask for any feedback you may have
> at
> > this time.
>
> thanks for proposing to bring weak references into core, I myself would
> have needed them once or twice.
>
> One point from the RFC bothers me:
>
> > The proposed API:
> > - does not support serialization
>
> That is a really unfortunate decision, I would really like to have them
> serializable. Otherwise if you want to persist a weak ref you always
> have to convert them to a real ref before serialization and back
> afterwards. I would really like to have this transparency build in.
> Would be great if that could be added.
>

Serialization for weak refs is a bit tricky, and I feel like supporting it
could easily become a foot-gun. The fundamental problem is that for
serialization to produce a meaningful value, the object referenced by a
WeakReference must also be part of the serialized object graph as a strong
reference somewhere. Otherwise, the WeakReference will immediately turn
into a dud when unserialization ends, because the object it references is
no longer live.

I think from a technical perspective, supporting serialization should be
possible and not overly hard, it's more a question of whether we want to.
As another data point, Java does not support serialization for
WeakReference.

Nikita

Dennis Birkholz

7 years ago

Hi Nikita,

On 05.02.19 16:50, Nikita Popov wrote:

> Serialization for weak refs is a bit tricky, and I feel like supporting
> it could easily become a foot-gun. The fundamental problem is that for
> serialization to produce a meaningful value, the object referenced by a
> WeakReference must also be part of the serialized object graph as a
> strong reference somewhere. Otherwise, the WeakReference will
> immediately turn into a dud when unserialization ends, because the
> object it references is no longer live.
>
> I think from a technical perspective, supporting serialization should be
> possible and not overly hard, it's more a question of whether we want
> to. As another data point, Java does not support serialization for
> WeakReference.

my fundamental problem here is that you can not reliably find out if
something can be serialized or not. Each class that is not serializable
by documentation/implementation but does not provide some programatical
way of checking this adds another burden to create a special case for
that class.

So I would really prefer the WeakReference class would just be
serializable with all the consequences you outlined. Maybe for PHP 8 we
will decide to require all classes to be serializable except classes
that implement the Unserializable interface or something... Or the magic
__isSerializable() method that can decide this at runtime for the actual
state of an object without failing hard when trying it.

Greets
Dennis

Girgias

7 years ago

On Tue, 5 Feb 2019 at 17:06, Dennis Birkholz <php@dennis.birkholz.biz>
wrote:

> So I would really prefer the WeakReference class would just be
> serializable with all the consequences you outlined. Maybe for PHP 8 we
> will decide to require all classes to be serializable except classes
> that implement the Unserializable interface or something... Or the magic
> __isSerializable() method that can decide this at runtime for the actual
> state of an object without failing hard when trying it.
>
> Greets
> Dennis
>

That on its own would need an RFC AFAIK and I'm not really a fan of this
hypothetical change.
Also it feels kind of backwards imho. Why should I specify that something
is NOT serializable
when serialisation is a behaviour that usually needs special care with?

Also what would happen when someone does the typical thing to prevent
serialization:
public function __sleep() { throw new Exception(); }

Best regards

George P. Banyard

Rowan Collins

7 years ago

On Tue, 5 Feb 2019 at 16:06, Dennis Birkholz <php@dennis.birkholz.biz>
wrote:

> Maybe for PHP 8 we
> will decide to require all classes to be serializable except classes
> that implement the Unserializable interface or something... Or the magic
> __isSerializable() method that can decide this at runtime for the actual
> state of an object without failing hard when trying it.
>

I agree, it can be very confusing if you have something like a SimpleXML
object buried somewhere, and your serialize() call blows up. (See the
example I've raised a couple of times where Exceptions may or may not be
serializable, because they slurp up objects from everywhere in their
backtrace.)

I was going to suggest this would fit with the discussion Nikita raised
about adding a new serialization mechanism, but thinking about it more, I'm
not sure how the API for this would work. If you don't recursively check
that the properties of an object are also serializable, the call is
useless; but if you do, it's really no different from try {
serialize($foo); } catch {}

Regards,

--
Rowan Collins
[IMSoP]