Review Request: Add ClassOption.STRONG and default is unspecified

[Mandy Chung]

On 3/11/20 3:34 PM, David Holmes wrote:
> 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. 

There are cases that the library code does not care.

Note that the main motivation to allow a hidden class be strongly 
referenced because of our current implementation.   It creates a 
ClassLoaderData per weak class that may result in fragmentation and 
incur bad performance overhead.

This is solely implementation detail.  Other VM implementation may favor 
weak classes.  An implementation-specific default will give flexibility 
to the developers (not to specify neither strong or weak) to choose 
whatever performs best when running on a VM implementation.

Mandy

> 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
>>>
>>