Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

Hi Peter,

On 4/4/20 3:58 AM, Peter Levart wrote:
>
> Here I think, you are not quite right. First I need to clarify that we 
> are talking about the case where the method reference in above example 
> is not converted to lambda by javac, so the proxy class needs to 
> invoke the superclass method directly (without the help of lambda 
> bridge). I did an experiment and compiled the example with JDK 13 
> javac, where the patch for (JDK-8234729) is not applied yet. What I 
> get from this compilation is the following metafactory bootstrap 
> method invocation:
>
>   0: #35 REF_invokeStatic 
> java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
>     Method arguments:
>       #42 ()V
>       #43 REF_invokeVirtual test/LambdaTest.m:()V
>       #42 ()V
>
> The #43 is the implMethod method handle and it is the following:
>
>   #43 = MethodHandle       5:#44          // REF_invokeVirtual 
> test/LambdaTest.m:()V
>   #44 = Methodref          #2.#45         // test/LambdaTest.m:()V
>   #45 = NameAndType        #46:#6         // m:()V
>   #46 = Utf8               m
>    #2 = Class              #4             // test/LambdaTest
>    #4 = Utf8               test/LambdaTest
>
> *BUT* the class that looks up this MH from the constant pool is the 
> subclass (test.sub.LambdaTestSub) which is equivalent to the following 
> programmatic lookup:
>
>         var mh = MethodHandles.lookup().findVirtual(LambdaTest.class, 
> "m", MethodType.methodType(void.class));
>         System.out.println(mh.type());
>
> and this results in a method handle of the following type: 
> (LambdaTestSub)void
>
> which is correct since the method handle *MUST* check that the passed 
> in instance (this) is of type LambdaTestSub or subtype or else the 
> "protected" access would be violated.
>
> And since the ref type is REF_invokeVirtual, the 
> AbstractValidatingLambdaMetafactory assigns the following to the 
> implClass:
>
>         this.implMethodType = implMethod.type();
>         this.implInfo = caller.revealDirect(implMethod);
>         switch (implInfo.getReferenceKind()) {
>             case REF_invokeVirtual:
>             case REF_invokeInterface:
>                 this.implClass = implMethodType.parameterType(0);
>
> ...which makes the implClass be LambdaTestSub in this case. Which is 
> correct again since we want InnerClassLambdaMetafactory to decide that 
> this is the special case for proxy to invoke the method via method 
> handle:
>
>         useImplMethodHandle = 
> !implClass.getPackageName().equals(implInfo.getDeclaringClass().getPackageName())
>                                 && 
> !Modifier.isPublic(implInfo.getModifiers());
>
> If implClass was test.LambdaTest as you said above, this condition 
> would evaluate to false, since implInfo is "invokeVirtual 
> test.LambdaTest.m:()void" in above case.
>

My bad.  I mixed up with implClass and implInfo cracked from implMethod 
in your question.

implInfo::getDeclaringClass() returns the declaring class of the 
resolved method, which is the superclass (which is what I have been 
thinking for test.LambdaTest).

implClass is the target type of the method reference. 
AbstractValidatingLambdaMetafactory has clear comment:

     final MethodType implMethodType;  // Type of the implMethod 
MethodHandle "(CC,int)String"
     final Class<?> implClass;                 // Class for referencing 
the implementation method "class CC"


> So everything is OK, but my original question was the following: The 
> name of the generated proxy class is derived from the targetClass 
> which is the caller's lookup class. In this example the caller is 
> LambdaTestSub and this is the same as implClass in this case.

Yes.

> Would those two classes always be the same in the case where the 
> evaluation of the above `useImplMethodHandle` boolean matters? I mean, 
> the decision about whether to base the proxy invocation strategy on 
> method handle or direct bytecode invocation is based on one class 
> (implClass), but the actual package of the proxy class which 
> effectively influences the bytecode invocation rights is taken from 
> another class (targetClass).
>
> On one hand the package of the proxy class has to be the same as 
> targetClass if the proxy wants to be the nestmate of the targetClass 
> (for example to have private access to the members of the nest). But 
> OTOH it needs to be the same package also with implClass so that the 
> above decision of the proxy invocation strategy is correct. I have a 
> feeling that for protected virtual methods, this is true, because the 
> type of the 0-argument of such method handle is always the lookup 
> class. But am I right?
>
> What do you think of another alternative to invoking the super 
> protected method in other package: What if the LMF would decide to 
> base the name of the proxy class on the implInfo.getDeclaringClass() 
> in such case? It would not have to be a nestmate of any class, just in 
> the same package with the method's declaring class. Consequently it 
> would be in the same module as the declaring class and loaded by the 
> same class loader. Therefore it would have to be WEAKLY referenced 
> from the class loader. And the Lookup instance passed to bootstrap LMF 
> method would not be enough for defining such class. But LMF could 
> obtain special powers since it is JDK internal class...
>

The implementation of the method reference invocation is logical part of 
the caller class.  I don't think spinning the proxy class in a remote 
package is the desirable thing to do (injecting a class from package A 
to package B).


> Well, I don't know for myself. Is this situation rare enough so that 
> invoking via method handle is not a drawback? It only happens when 
> running JDK 13- compiled classes with JDK 15+ and in addition the 
> method reference must point to protected method in a distant class.

There are other alternatives we considered.  This implementation is a 
just short term solution.  We plan to follow up some future enhancements 
that are the possible longer-term solutions for this issue.

1. JDK-8239580 evaluate the performance of direct method handle 
invocation rather than bytecode invocation for ALL cases

Direct invocation of the `implMethod` method handle by the lambda proxy 
was explored in JDK 8 lambda development time. It is time to remeasure 
the performance of direct method handle invocation and re-evaluate that 
approach.

2.  JDK-8230501 class data support.  The live MethodHandle can be passed 
to the hidden class to replace the current implementation to set the 
implMethod in a static field of proxy class after it's defined.

3. JDK-8199386 enhance Lookup::in to support nestmates

Hope this helps.
Mandy

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200404/696247b3/attachment.htm>