On Tue, 27 Jun 2023 16:33:11 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 updates the WrapperInstance methods to take an `Empty` to avoid method 
>> clashes
>> 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 faster than old implementation in general.
>> 
>> Benchmark for revision 17:
>> 
>> Benchmark                                                          Mode  Cnt 
>>      Score       Error  Units
>> MethodHandleProxiesAsIFInstance.baselineAllocCompute               avgt   15 
>>      1.503 ±     0.021  ns/op
>> MethodHandleProxiesAsIFInstance.baselineCompute                    avgt   15 
>>      0.269 ±     0.005  ns/op
>> MethodHandleProxiesAsIFInstance.testCall                           avgt   15 
>>      1.806 ±     0.018  ns/op
>> MethodHandleProxiesAsIFInstance.testCreate                         avgt   15 
>>     17.332 ±     0.210  ns/op
>> MethodHandleProxiesAsIFInstance.testCreateCall                     avgt   15 
>>     19.296 ±     1.371  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callDoable                     avgt    5 
>>      0.419 ±     0.004  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callHandle                     avgt    5 
>>      0.421 ±     0.004  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callInterfaceInstance          avgt    5 
>>      1.731 ±     0.018  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callLambda                     avgt    5 
>>      0.418 ±     0.003  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantDoable                 avgt    5 
>>      0.263 ±     0.003  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantHandle                 avgt    5 
>>      0.262 ±     0.002  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantInterfaceInstance      avgt    5 
>>      0.262 ±     0.002  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantLambda                 avgt    5 
>>      0.267 ±     0.019  ns/op
>> MethodHandleProxiesAsIFInstanceCall.direct                         avgt    5 
>>      0.266 ±     0.013  ns/op
>> MethodHandleProxiesAsIFInstanceCreate.createCallInterfaceInstance  avgt    5 
>>     18.057 ±     0.182 ...
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 38 commits:
> 
>  - SecurityManager fixed, minimize changes
>  - Merge branch 'master' into explore/mhp-iface
>  - Some changes per Mandy's review. SecurityManager test fails in this patch
>  - Merge branch 'master' into explore/mhp-iface
>  - Merge branch 'master' into explore/mhp-iface
>  - Merge branch 'master' into explore/mhp-iface
>  - stage, needs to fix is mh proxy instance check
>  - Merge branch 'master' into explore/mhp-iface
>  - Remove assertion, no longer true with teleport definition in MHP
>  - Fix tabs, and tests about modules and constructor access
>  - ... and 28 more: https://git.openjdk.org/jdk/compare/05e9c41e...07cc1279

I have updated this patch again, this time updated to latest Classfile API and 
removed irrelevant changes.

1. The previous SecurityManager failure was because of 
ClassLoaders$AppClassLoader's security manager package access check. Even if we 
can patch the package access check there, we cannot rule out that another 
interface's class loader reject loading jdk.internal classes specifically 
exported to our dynamic module. Thus, I moved the check method to be part of 
the class template, and chose the ClassValue approach instead of using a marker 
interface.
2. Given this observation, exporting jdk.internal packages to modules with user 
class loaders may be problematic; we might need our custom class loaders if we 
do export jdk.internal content to a class that can see user classes.

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

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

Reply via email to