On Wed, 18 Nov 2020 00:50:22 GMT, Mandy Chung <mch...@openjdk.org> wrote:

>> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live 
>> objects
>> be shared between a hidden class and other classes.  A hidden class can load
>> these live objects as dynamically-computed constants via this API.
>> 
>> Specdiff
>> http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html
>> 
>> With this class data support and hidden classes, 
>> `sun.misc.Unsafe::defineAnonymousClass`
>> will be deprecated for removal.  Existing libraries should replace their
>> calls to `sun.misc.Unsafe::defineAnonymousClass` with 
>> `Lookup::defineHiddenClass`
>> or `Lookup::defineHiddenClassWithClassData`.
>> 
>> This patch also updates the implementation of lambda meta factory and
>> `MemoryAccessVarHandleGenerator` to use class data.   No performance 
>> difference
>> observed in the jdk.incubator.foreign microbenchmarks.   A side note: 
>> `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of 
>> JDK-8254162 but it helps validating the class data support.
>> 
>> Background
>> ----------
>> 
>> This is an enhancement following up JEP 371: Hidden Classes w.r.t.
>> "Constant-pool patching" in the "Risks and Assumption" section.
>> 
>> A VM-anonymous class can be defined with its constant-pool entries already
>> resolved to concrete values. This allows critical constants to be shared
>> between a VM-anonymous class and the language runtime that defines it, and
>> between multiple VM-anonymous classes. For example, a language runtime will
>> often have `MethodHandle` objects in its address space that would be useful
>> to newly-defined VM-anonymous classes. Instead of the runtime serializing
>> the objects to constant-pool entries in VM-anonymous classes and then
>> generating bytecode in those classes to laboriously `ldc` the entries,
>> the runtime can simply supply `Unsafe::defineAnonymousClass` with references
>> to its live objects. The relevant constant-pool entries in the newly-defined
>> VM-anonymous class are pre-linked to those objects, improving performance
>> and reducing footprint. In addition, this allows VM-anonymous classes to
>> refer to each other: Constant-pool entries in a class file are based on 
>> names.
>> They thus cannot refer to nameless VM-anonymous classes. A language runtime 
>> can,
>> however, easily track the live Class objects for its VM-anonymous classes and
>> supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new 
>> class's
>> constant pool entries to other VM-anonymous classes.
>> 
>> This extends the hidden classes to allow live objects to be injected
>> in a hidden class and loaded them via condy.
>> 
>> Details
>> -------
>> 
>> A new `Lookup::defineHiddenClassWithClassData` API takes additional
>> `classData` argument compared to `Lookup::defineHiddenClass`.
>> Class data can be method handles, lookup objects, arbitrary user objects
>> or collections of all of the above.
>> 
>> This method behaves as if calling `Lookup::defineHiddenClass` to define
>> a hidden class with a private static unnamed field that is initialized
>> with `classData` at the first instruction of the class initializer.
>> 
>> `MethodHandles::classData(Lookup lookup, String name, Class<?> type)` and
>> `MethodHandles::classDataAt(Lookup lookup, String name, Class<?> type, int 
>> index)`
>> are the bootstrap methods to load the class data of the given lookup's 
>> lookup class.
>> The hidden class will be initialized when `classData` method is called if
>> the hidden class has not been initialized.
>> 
>> For a class data containing more than one single element, libraries can
>> create their convenience method to load a single live object via condy.
>> 
>> Frameworks sometimes want to dynamically create a hidden class (HC) and add 
>> it
>> it the lookup class nest and have HC to carry secrets hidden from that nest.
>> In this case, frameworks should not to use private static finals (in the HCs
>> they spin) to hold secrets because a nestmate of HC may obtain access to
>> such a private static final and observe the framework's secret.  It should 
>> use
>> condy.  In addition, we need to differentiate if a lookup object is created 
>> from
>> the original lookup class or created from teleporting e.g. `Lookup::in`
>> and `MethodHandles::privateLookupIn`.
>> 
>> This proposes to add a new `ORIGINAL` bit that is only set if the lookup
>> object is created by `MethodHandles::lookup` or by bootstrap method 
>> invocation.
>> The operations only apply to a Lookup object with original access are:
>>    - create method handles for caller-sensitve methods
>>    - obtain class data associated with the lookup class
>> 
>> No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which
>> ignores the ORIGINAL bit.
>> 
>> 
>> Compatibility Risks
>> -------------------
>> 
>> `Lookup::lookupModes` includes a new `ORIGINAL` bit.  Most lookup operations
>> ignore this original bit except creating method handles for caller-sensitive 
>> methods
>> that expects the lookup from the original lookup class.  Existing code  
>> compares
>> the return value of `lookupModes` to be a fixed value may be impacted.   
>> However
>> existing client has no need to expect a fixed value of lookup modes.
>> The incompatibility risk of this spec change is low.
>
> Mandy Chung has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix the name passed to condy calling classData

IIUC `classData` can be used for an original lookup that is not produced by the 
result of `defineHiddenClassWithClassData`, but in such cases the class data 
will always be null.

Since `defineHiddenClassWithClassData` rejects null values for class data, we 
could detect such usage and throw in the bootstrap methods. That would require 
a special constant assignment for hidden classes with no class data. Probably 
not worth it.

Recommend an API note.

src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 342:

> 340:          } catch (ClassCastException e) {
> 341:              throw e;
> 342:          } catch (Throwable e) {

The following might be more appropriate so, in general, errors and runtime 
exceptions are not explicitly wrapped:

        try {
            return BootstrapMethodInvoker.widenAndCast(classdata, type);
        }
        catch (RuntimeException | Error e) {
            throw e;
        }
        catch (Throwable e) {
            throw new InternalError("Unexpected exception", e);
        }

same applies to `classDataAt` and `ConstantBootstraps.explicitCast`. Refinement 
of the runtime exceptions is also possible, but i think the key thing here is 
to let errors pass through and any possibly expected runtime exceptions will 
get wrapped in `BootstrapMethodError`.

test/jdk/java/lang/invoke/MethodHandles/classData/ClassDataTest.java line 77:

> 75:      */
> 76:     @Test
> 77:     public void noClassData() throws Throwable {

`throws Throwable` needed on this and other method declarations?

-------------

Marked as reviewed by psandoz (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1171

Reply via email to