[constant-folding] More indy experiments

[Maurizio Cimadamore]

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.

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


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.