Review Request: Add ClassOption.STRONG and default is unspecified

[Alex Buckley]

(Responding even though the enum constants are slightly up in the air, 
because ultimately we need to be able to characterize unloading of a 
hidden class independently of loader implementation.)

I like how the first paragraph sidesteps how the loader implementation 
refers to the hidden class. However, the second paragraph needs to keep 
up the good work when no option is given.

-----
***Unloading of the newly created class or interface {@code C} may be 
controlled via the {@link ClassOption#WEAK WEAK} and {@link 
ClassOption#STRONG STRONG} options.*** If {@code options} has the {@code 
WEAK} option, then {@code C} may be unloaded when it [DO WE MEAN ITS 
CLASS OBJECT?] becomes unreachable and is ~~~being~~~ reclaimed by the 
garbage collector***,*** regardless of the <a 
href="../ref/package.html#reachability">reachability</a> state of {@code 
C}'s defining class loader. If {@code options} has ***the*** {@code 
STRONG} option, then {@code C} is guaranteed not to be unloaded when its 
defining class loader is strongly reachable.

At most one of the options {@code WEAK} and {@code STRONG} may be given. 
If neither is given, then the relationship between the defining loader's 
reachability and the hidden class's unloading is unspecified. In this 
case, the loader implementation [now we can talk about references] may 
use a strong ...
-----


BTW, the items which determine the nest host of C in the NESTMATE case 
should look like this: (note the ol):

-----
<ol>

<li>If the nest host of the lookup class of this {@code Lookup} has 
previously been determined, then ***let*** {@code H} be the nest host of 
the lookup class. Otherwise, the nest host of the lookup class is 
determined using the algorithm in JVMS 5.4.4, yielding {@code H}.</li>

<li>The nest host of {@code C} is determined to be {@code H}, the nest 
host of the lookup class.</li>

</ol>
-----

Alex

On 3/11/2020 3:12 PM, 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.
> 
> 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
>>
>