[constant-folding] More indy experiments

[Vicente Romero]

Hi Tagir,

Thanks for the test and the bug report. Did you signed the OCA?

On 05/10/2017 05:55 AM, Maurizio Cimadamore wrote:
> Hi Tagir,
> thanks for testing the prototype.
>
> As you noticed, class literal support has been dropped; we're 
> discussing the pros and cons of this internally, we will let you know 
> the outcome of that discussion soon.
>
> As for your issue, (1) is clearly a bug - albeit not sure if it's in 
> the compiler or the BSM runtime - Vicente please take a look.

I have just pushed: 
http://hg.openjdk.java.net/amber/amber/langtools/rev/31adff18583d, which 
takes care of issuing a compiler error if an empty string is passed as 
the value of the invocationName argument for invokedynamic(). I'm also 
investigating the reason for 3)

>
> As for (2) - the extra boxing you are observing is caused by the fact 
> that Intrinsic.invokedynamic is not (yet) a polymorphic signature 
> method (note that the polysig annotation is mentioned in the JEP). 
> When it will be, boxing will disappear. But I think that if we go down 
> the polysig path, then an explicit cast for the return type would be 
> necessary (as it is for i.e. MethodHandle.invokeExact).
>
> Maurizio

Thanks,
Vicente

>
>
> On 10/05/17 07:33, Tagir Valeev wrote:
>> Hello!
>>
>> I tried to make more sophisticated example with the latest code.
>> Unfortunately seems that using Class as polyexpression is not supported
>> anymore which adds more boilerplate to the code. Nevertheless here's my
>> attempt and several issues I've found.
>>
>> The problem: make a dynamic callsite which multiplies two longs and 
>> returns
>> BigInteger. At first it should optimistically think that 
>> multiplication of
>> two longs will not overflow. However if it overflows once at given call
>> site, it switches to the slower implementation.
>>
>> Here's the code (written by me, can be used under OCA terms if you 
>> wish to
>> use it as testcase, etc.):
>>
>> import java.lang.invoke.*;
>> import java.math.BigInteger;
>> import java.lang.invoke.Constables.*;
>>
>> public class IndyTest {
>> // library code starts
>>    static class MultiplyCallSite extends MutableCallSite {
>>      private static final MethodTypeConstant TYPE = 
>> MethodTypeConstant.of(
>>        ClassConstant.of("Ljava/math/BigInteger;"), 
>> ClassConstant.of("J"),
>> ClassConstant.of("J"));
>>      private static final ClassConstant ME =
>> ClassConstant.of("LIndyTest$MultiplyCallSite;");
>>
>>      private static final MethodHandle FAST =
>>        Intrinsics.ldc(MethodHandleConstant.ofVirtual(ME, "fast", TYPE));
>>      private static final MethodHandle SLOW =
>>        Intrinsics.ldc(MethodHandleConstant.ofStatic(ME, "slow", TYPE));
>>
>>      MultiplyCallSite(MethodType type) {
>>        super(type);
>>        setTarget(FAST.bindTo(this).asType(type));
>>      }
>>
>>      BigInteger fast(long a, long b) {
>>        try {
>>          return BigInteger.valueOf(Math.multiplyExact(a, b));
>>        } catch (ArithmeticException ex) {
>>          // switch to slower implementation
>>          setTarget(SLOW.asType(type()));
>>          return slow(a, b);
>>        }
>>      }
>>
>>      static BigInteger slow(long a, long b) {
>>        return BigInteger.valueOf(a).multiply(BigInteger.valueOf(b));
>>      }
>>    }
>>
>>    public static final BootstrapSpecifier MULT =
>> BootstrapSpecifier.of(MethodHandleConstant.ofStatic(
>>      ClassConstant.of("LIndyTest;"), "multiplyFactory",
>>
>> MethodTypeConstant.of("(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;"))); 
>>
>>
>>    public static CallSite multiplyFactory(MethodHandles.Lookup lookup,
>> String name, MethodType type) {
>>      return new MultiplyCallSite(type);
>>    }
>> // library code ends
>>
>>    public static BigInteger multiplyIndy(long bigNum, long smallNum) {
>>      try {
>>        return ((BigInteger) Intrinsics.invokedynamic(MULT, " ", bigNum,
>> bigNum))
>>            .add((BigInteger) Intrinsics.invokedynamic(MULT, " ", 
>> smallNum,
>> smallNum));
>>      } catch (Throwable throwable) {
>>        throw new InternalError(throwable);
>>      }
>>    }
>>
>>    public static void main(String[] args) {
>>      System.out.println(multiplyIndy(5000000000L, 6));
>>    }
>> }
>>
>> ---
>> This indeed works. A few problems though:
>>
>> 1). I don't need the name parameter of the factory method. When I pass
>> there an empty string (not single space like in the code above), I 
>> have an
>> incorrect bytecode:
>>
>> Error: LinkageError occurred while loading main class IndyTest
>> java.lang.ClassFormatError: Illegal zero length constant pool entry 
>> at 91
>> in class IndyTest
>>
>> I don't understand exactly what happens. Probably it's incorrectly 
>> encoded
>> in constant pool or BSM does not allow empty string as name argument.
>> Nevertheless it should either work or end up with a compilation 
>> error, not
>> runtime error.
>>
>> Also I think that name is not always useful and it would be fine to 
>> be able
>> to omit it (or hide inside BootstrapSpecifier like it was done before).
>>
>> 2). It boxes arguments unnecessarily. Here's start of multiplyIndy 
>> method
>> bytecode:
>>
>> 0: lload_0
>> 1: invokestatic  #4                  // Method
>> java/lang/Long.valueOf:(J)Ljava/lang/Long;
>> 4: lload_0
>> 5: invokestatic  #4                  // Method
>> java/lang/Long.valueOf:(J)Ljava/lang/Long;
>> 8: invokedynamic #5,  0              // InvokeDynamic
>> #0:"":(Ljava/lang/Long;Ljava/lang/Long;)Ljava/math/BigInteger;
>>
>> I'd expect to see here
>>
>> 0: lload_0
>> 1: lload_0
>> 2: invokedynamic #5,  0              // InvokeDynamic
>> #0:"":(JJ)Ljava/math/BigInteger;
>>
>> 3). It does not merge equal BSM attributes. Resulting class has two 
>> of them:
>>
>> BootstrapMethods:
>>    0: #61 REF_invokeStatic
>> IndyTest.multiplyFactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite; 
>>
>>      Method arguments:
>>    1: #61 REF_invokeStatic
>> IndyTest.multiplyFactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite; 
>>
>>      Method arguments:
>>
>> I would expect to have only one.
>>
>> A side thought:
>>
>> Probably it would be good to bind BootstrapSpecifier to the return 
>> type to
>> omit explicit casts of return value. The signature of invokedynamic 
>> would
>> be then:
>>
>> static <T> T invokedynamic(BootstrapSpecifier<T> bsm, ...) {..}
>>
>> This way I would be able to write:
>>
>> // in library code
>> public static final BootstrapSpecifier<BigInteger> MULT =
>> BootstrapSpecifier.of(...);
>>
>> // in user code:
>> return Intrinsics.invokedynamic(MULT, " ", bigNum, bigNum)
>>            .add(Intrinsics.invokedynamic(MULT, " ", smallNum, 
>> smallNum));
>>
>> So this would be a library responsibility to specify an expected return
>> type which would reduce possible errors in user code. Of course
>> BootstrapSpecifier<?> could be used if specific return type is not
>> applicable for some reason.
>>
>> Thank you for the great work!
>> With best regards,
>> Tagir Valeev.
>