Re: [PHP-DEV] New gc_status() fields, and WeakMap cycle collection

2023-07-07 Thread Arnaud Le Blanc
I agree that we may not consider this a BC break, but I'm asking in
case people rely on the current behavior or if the change is more
controversial than I thought.

To be fair they work as specified in the RFC, and are actually weak as
they do not explicitly retain their keys, but they need explicit
support from the GC to be able to collect some types of cycles.
WeakMap implementations based on WeakReferences tend to have this
limitation. For instance, the Java documentation for WeakHashMap has
this warning: "The value objects in a WeakHashMap are held by ordinary
strong references. Thus care should be taken to ensure that value
objects do not strongly refer to their own keys, either directly or
indirectly, since that will prevent the keys from being discarded".


On Fri, Jul 7, 2023 at 4:03 PM Larry Garfield  wrote:
>
> On Fri, Jul 7, 2023, at 1:09 PM, Arnaud Le Blanc wrote:
> > Hi everyone,
> >
> > I'm considering merging the following two PRs:
> >
> > https://github.com/php/php-src/pull/11523: Exposes the time spent in
> > the cycle collector via new gc_status() fields.
> > https://github.com/php/php-src/pull/10932: Allows the GC to collect
> > certain types of cycles involving WeakMaps.
> >
> > The second one introduces a minor BC break.
> >
> > If there are no objections, I plan to merge these PRs prior to the
> > feature freeze.
> >
> > Best regards,
> > Arnaud
>
> For the second, doesn't that translate to "WeakMaps are currently not 
> actually Weak, and thus don't work at all?"  I don't quite know how that 
> would be a BC break to fix...
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] New gc_status() fields, and WeakMap cycle collection

2023-07-07 Thread Larry Garfield
On Fri, Jul 7, 2023, at 1:09 PM, Arnaud Le Blanc wrote:
> Hi everyone,
>
> I'm considering merging the following two PRs:
>
> https://github.com/php/php-src/pull/11523: Exposes the time spent in
> the cycle collector via new gc_status() fields.
> https://github.com/php/php-src/pull/10932: Allows the GC to collect
> certain types of cycles involving WeakMaps.
>
> The second one introduces a minor BC break.
>
> If there are no objections, I plan to merge these PRs prior to the
> feature freeze.
>
> Best regards,
> Arnaud

For the second, doesn't that translate to "WeakMaps are currently not actually 
Weak, and thus don't work at all?"  I don't quite know how that would be a BC 
break to fix...

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] New gc_status() fields, and WeakMap cycle collection

2023-07-07 Thread Arnaud Le Blanc
Hi everyone,

I'm considering merging the following two PRs:

https://github.com/php/php-src/pull/11523: Exposes the time spent in
the cycle collector via new gc_status() fields.
https://github.com/php/php-src/pull/10932: Allows the GC to collect
certain types of cycles involving WeakMaps.

The second one introduces a minor BC break.

If there are no objections, I plan to merge these PRs prior to the
feature freeze.

Best regards,
Arnaud

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php