[External] : Re: Minor improvement to anonymous classes

[Brian Goetz]

I think you’ve done pretty good due diligence here.  One more thing we could do is reach out to the most popular libraries that do this and give them a heads up that they need to tolerate the field not being there. But overall, the benefit accrues to the 99.999% of users that follow the rules.

Sent from my iPad

On Oct 6, 2021, at 2:15 PM, Liam Miller-Cushon <cushon at google.com> wrote:


Belatedly returning to this, +Joe Darcy<mailto:joe.darcy at oracle.com> helped with some corpus analysis in the CSR [1] (thanks!). The analysis didn't reveal any build breakages from optimizing away this$0, but it did reveal hundreds of textual occurrences of this$0. The behaviour of those occurrences of this$0 could potentially change if code is reflectively accessing the enclosing instance, and if it expects to be able to do that even if the inner class doesn't capture any enclosing instance state.

As I mentioned earlier we've been using a version of the patch at Google since 2016 [2]. Rolling it out required a very small amount of cleanup, and I am not aware of it causing any issues since then (including with third party libraries that might have been relying on the hack). We have some remaining occurrences of this$0 in our code, which are not affected by the change because the only need to handle this$0 in classes that actually capture their enclosing instance.

So from my (admittedly limited) perspective, this change is beneficial, and has minor compatibility impact relative to the other breaking changes we've absorbed.

I'm curious if anyone has suggestions about how to get other data or perspectives that might help decide how to proceed here?

If we can't get conclusive information on how much code would be affected by this, maybe it would be sufficient to roll the change out more conservatively, e.g. by only enabling it for new language levels? Has the 'preview feature' mechanism ever been used for things like this, or is intended more for new features that are visible in the spec?

[1] https://bugs.openjdk.java.net/browse/JDK-8271717?focusedCommentId=14442858&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14442858
[2] https://bugs.openjdk.java.net/browse/JDK-8271623?focusedCommentId=14439152&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14439152

On Tue, Aug 3, 2021 at 5:41 AM Brian Goetz <brian.goetz at oracle.com<mailto:brian.goetz at oracle.com>> wrote:
Yes, local classes too.  Essentially, this is for translation of
"effectively static" inner classes.

I think this is independent of explicit-static or not; explicit-static
allows the programmer to capture intent and get more type checking as a
result.  This is about generating better code.

On 8/3/2021 12:52 AM, Tagir Valeev wrote:
> Another possible semantics change is the object lifetime. The code
> might rely on prolonged lifetime of the surrounding object if there
> are soft/weak/phantom references. E.g., the outer object might be
> registered via Cleaner, and the change may cause freeing the resource
> earlier than expected. Likely, this is a very rare scenario but if it
> happens, it could be quite hard to identify the root cause, as the
> problem will appear only if the object is collected within the
> specific timeframe.
>
> By the way, are we speaking about anonymous classes only? I think,
> local classes could be updated in the similar manner. Especially given
> the fact that now local records don't capture the surrounding "this"
> but if we convert the record to an equivalent local class, it will
> capture:
>
> public class Test {
>    void test() {
>      record R() {} // does not capture Test instance
>      class C {} // captures Test instance
>    }
> }
>
> Or should we allow explicit 'static' modifier on local classes?
>
> Best regards,
> Tagir Valeev.
>
> On Tue, Aug 3, 2021 at 2:47 AM <forax at univ-mlv.fr<mailto:forax at univ-mlv.fr>> wrote:
>> We may have some trouble with the usual suspect, Serialization,
>> There are classes like exceptions or Swing UI classes that are marked as Serializable and can be implemented as an anonymous class.
>> In that case, removing the backpointer if it is not used may change the serialization format.
>>
>> And yes, an anonymous class do not have a "stable" name but people do not seem to care too much about that ...
>>
>> Rémi
>>
>> ----- Original Message -----
>>> From: "Brian Goetz" <brian.goetz at oracle.com<mailto:brian.goetz at oracle.com>>
>>> To: "Liam Miller-Cushon" <cushon at google.com<mailto:cushon at google.com>>
>>> Cc: "Remi Forax" <forax at univ-mlv.fr<mailto:forax at univ-mlv.fr>>, "John Rose" <john.r.rose at oracle.com<mailto:john.r.rose at oracle.com>>, "amber-spec-experts"
>>> <amber-spec-experts at openjdk.java.net<mailto:amber-spec-experts at openjdk.java.net>>
>>> Sent: Lundi 2 Août 2021 20:18:56
>>> Subject: Re: [External] : Re: Minor improvement to anonymous classes
>>> FWIW, making this fix not only reduces the memory leak risk, but has a
>>> number of nice follow-on benefits that can often trigger further
>>> follow-on benefits:
>>>
>>>   - fewer fields, so reduced footprint;
>>>   - fewer fields might mean more objects fall under the scalarization
>>> threshold, when applicable;
>>>   - less work in constructors;
>>>   - shorter constructors mean more constructors fall under the inlining
>>> threshold;
>>>   - more inlining might lead to other optimizations.
>>>
>>> So it wouldn't surprise me to see macro-level effects even on programs
>>> without memory leaks.
>>>
>>>> I filed https://bugs.openjdk.java.net/browse/JDK-8271623
>>>> <https://bugs.openjdk.java.net/browse/JDK-8271623> to track that
>>>> enhancement.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/amber-spec-experts/attachments/20211006/18e0b4d1/attachment-0001.htm>