Review Request: Add ClassOption.STRONG and default is unspecified

[David Holmes]

Hi Mandy,

On 12/03/2020 8:12 am, Mandy Chung wrote:
> Webrev:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/weak-strong-class/ 
> 
> 
> Alex suggests to keep the default unspecified, which I think it's a 
> great suggestion such that people do not have to rely on the 
> implementation assumption.

I think this is making things unnecessarily awkward. The suggestion from 
yesterday (as per below) was to change the default to be weak, and allow 
people to ask for strong. That would involve simply changing all WEAK to 
STRONG through the existing code. Now we have WEAK and STRONG that are 
mutually exclusive so have to check for them both being set, and if 
neither is set then we fall into implementation-specific behaviour.

If it is insisted that we have both WEAK and STRONG then we can achieve 
having the default unspecified by requiring that one of them always be 
passed in. But I much prefer what was suggested previously: either 
default is strong and you can ask for weak; or default is weak and you 
can ask for strong.

Sorry.

David

> This patch adds a new ClassOption::STRONG enum constant while keeping 
> ClassOption::WEAK.

> Mandy
> 
> On 3/10/20 9:55 PM, John Rose wrote:
>> I agree with Alex.  Class loaders map names to classes.  It’s not
>> natural *semantically* for a nameless class to have its lifetime
>> determined by a class loader.  It may be more natural *in our
>> JVM implementation* (hence more performant, in the *current*
>> implementation) to have a non-weak link, but let’s do this only
>> if users request it explicitly.
>>
>> On Mar 10, 2020, at 4:55 PM, Mandy Chung <mandy.chung at oracle.com 
>> <mailto:mandy.chung at oracle.com>> wrote:
>>>
>>> Brian, John, et al,
>>>
>>> Alex has given his feedback on the weak class feature (attached).
>>>
>>> Alex recommends to change the default of hidden classes be weak 
>>> classes and introduce a ClassOption::STRONG option if it wants to 
>>> ensure that the hidden class is not being reclaimed until the class 
>>> loader becomes unreachable.  See more details below.
>>>
>>> We tried to keep a hidden class be a normal class from JVM's point of 
>>> view.  Classes in the VM implementation have the same lifecycle as 
>>> the defining class loader. In addition, LambdaForms and Nashorn are 
>>> the only JDK use of hidden weak classes.  All others such as lambda 
>>> metafactory and string concat use hidden (strong) classes as they are 
>>> part of the target class' internal implementation and should be 
>>> unloaded altogether. Hence strong reference to hidden classes is the 
>>> default.
>>>
>>> What's your recommendation?
>>>
>>> Mandy
>>>
>>>
>>> -------- Forwarded Message --------
>>> Subject:     Weak references to hidden classes
>>> Date:     Tue, 10 Mar 2020 16:16:50 -0700
>>> From:     Alex Buckley <alex.buckley at oracle.com>
>>> Organization:     Oracle Corporation
>>> To:     Mandy Chung <mandy.chung at oracle.com>
>>>
>>>
>>>
>>> JEP 371 says that a hidden class may be weakly referenced by its 
>>> class loader. Which loader? The only loader with any connection to a 
>>> hidden class is the loader nominated to serve as the defining loader 
>>> by the API spec of defineHiddenClass. However, that spec doesn't say 
>>> "The defining loader holds a strong reference to the hidden class" 
>>> because no spec says anything about how a class loader refers to a 
>>> class.
>>>
>>> (The only relationship we do specify is the other way around -- 
>>> ClassLoader says "Every Class object contains a reference to the 
>>> ClassLoader that defined it." -- and happily that relationship holds 
>>> for a hidden class too.)
>>>
>>> Point is, it's odd for defineHiddenClass to standardize how a class 
>>> loader refers to a hidden class when, conceptually, class loaders are 
>>> not meant to "know" about hidden classes.
>>>
>>> (The connection between a defining loader and a hidden class is a 
>>> mere accounting trick done to support the hidden class's own code. No 
>>> class loader, not even that defining loader, knows how to respond to 
>>> a class load request for the hidden class.)
>>>
>>> It's especially odd for defineHiddenClass's default to be a strong 
>>> reference. That default aligns with ClassLoader::defineClass, the API 
>>> we're trying to avoid, and doesn't align with 
>>> Unsafe::defineAnonymousClass, the API we're trying to recreate!
>>>
>>> I understand there are performance reasons to want a loader to use a 
>>> strong reference rather than a weak reference. Accepting that claim, 
>>> I recommend having the implementation use a weak reference by 
>>> default, having the spec allow that, then introducing 
>>> ClassOption.STRONG to force a strong reference. That is, specify that:
>>>
>>> "In the absence of an option, the defining loader's relationship with 
>>> the hidden class is unspecified, and the loader implementation may 
>>> use a strong reference or a weak reference to keep track of the 
>>> hidden class. If a hidden class is not strongly referenced by the 
>>> class loader which is deemed to be its defining loader, then the 
>>> hidden class can be unloaded when library code no longer refers to 
>>> it. Library developers who wish to prevent the hidden class from 
>>> being unloaded must ensure that its Class object is never reclaimed, 
>>> or else use the ClassOption.STRONG option."
>>>
>>> I understand there is a concern that developers would not realize / 
>>> be surprised by getting a weak reference as the default 
>>> implementation. However, the planet-wide audience for 
>>> defineHiddenClass is perhaps 500 developers. They already have to 
>>> know what they're doing because the features of hidden classes are 
>>> already more limited than the features of VM-anonymous classes., 
>>> e.g., no constant pool patching. The usual "Java should save 
>>> developers from making silly mistakes" rule doesn't apply here.
>>>
>>> Alex
>>
>