On Fri, 7 Apr 2023 14:35:41 GMT, Chen Liang <li...@openjdk.org> wrote:

>> As John Rose has pointed out in this issue, the current j.l.r.Proxy based 
>> implementation of MethodHandleProxies.asInterface has a few issues:
>> 1. Exposes too much information via Proxy supertype (and WrapperInstance 
>> interface)
>> 2. Does not allow future expansion to support SAM[^1] abstract classes
>> 3. Slow (in fact, very slow)
>> 
>> This patch addresses all 3 problems:
>> 1. It implements proxies with one hidden class for each requested interface 
>> and replaced WrapperInstance inheritance with a check to the class data. 
>> This can avoid unexpected passing of `instanceof`, and avoids the nasty 
>> problem of exporting a JDK interface to a dynamic module to ensure access.
>> 2. This patch obtains already generated classes from a ClassValue by the 
>> requested interface type; the ClassValue can later be updated to compute 
>> implementation generation for abstract classes as well.
>> 3. This patch's generated hidden classes has call performance on par with 
>> those of lambda expressions; the creation performance is slightly less than 
>> that of LambdaMetafactory: 
>> https://jmh.morethan.io/?gist=fcb946d83ee4ac7386901795ca49b224
>> 
>> Additionally, an obsolete `ProxyForMethodHandle` test was removed, for it's 
>> no longer applicable. Tests in `jdk/java/lang/invoke` and 
>> `jdk/java/lang/reflect` pass.
>> 
>> [^1]: single abstract method
>
> Chen Liang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix test that depend on WrapperInstance

I considered more on the membership of the MH proxy class and also about the 
boot loader being the defining class loader.  I would propose to fully 
encapsulate the MH proxy class as all other JDK internal classes.  

For each interface I and target MH, define a hidden MH proxy class in a dynamic 
module (similar to java.lang.reflect.Proxy).  The MH proxy class is fully 
encapsulated, i.e. its package is not exported and not open.  It has only 
access to the types in the package of the interface as well as the types 
referenced by the interface.   The types  accessible by the dynamic module are 
minimized even if it's defined by the boot loader.

This way, I think we can use the class loader of the given interface to define 
the hidden class.     The class file is generated once for each interface.  For 
each invocation of MethodHandleProxies::asInterfaceInstance(I, MH), a hidden 
class is defined with MH as the class data from the bytes of the generated 
class file for `I`.  So each dynamic module has one or more hidden classes 
defined.

This is my patch to implement this change and also include a few comments to 
your patch (e.g. rename the dumper's property name and also the proxy class 
should not be a nested class of the interface).
    
https://github.com/liachmodded/jdk/compare/explore/mhp-iface...mlchung:mh-proxies

This should add tests to cover the interface in a module that references types 
in another module.  

I won't be able to review this PR until end of May.

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

PR Comment: https://git.openjdk.org/jdk/pull/13197#issuecomment-1528138719

Reply via email to