[9] RFR (M): 8027827: Improve performance of catchException combinator

[Vladimir Ivanov]

Paul,

Thanks for the feedback! See my answers inline.

Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.01/

I finally figured out how to make caching work. This webrev contains 
these changes.

I changed LF representation a bit and added 2 auxiliary method handles 
- argument boxing and wrapping into Object[] and result unboxing. These 
operations depend on actual type and can't be shared among arbitrary 
combinators with the same basic type. They are used only during LF 
interpretation and are completely ignored in compiled LFs.

On 2/20/14 7:51 PM, Paul Sandoz wrote:
> Hi Vladimir,
>
> I know just enough about this area to be dangerous....
>
>
> src/share/classes/java/lang/invoke/BoundMethodHandle.java
>
>   865     private static final SpeciesData[] SPECIES_DATA_CACHE = new SpeciesData[4];
>
>
> Only 3 elements are stored in the above array?
Fixed.

>
>
>
> src/share/classes/java/lang/invoke/MethodHandleImpl.java
>
>   634         // t_{i+2}:L=ValueConversions.unbox(t_{i+1}) OR ValueConversions.identity(t_{i+1})
>   635         if (type.returnType().isPrimitive()) {
>   636             names[UNBOX_RESULT] = new Name(ValueConversions.unbox(type.returnType()),
>   637                                               names[TRY_CATCH]);
>   638         } else {
>   639             names[UNBOX_RESULT] = new Name(ValueConversions.identity(),
>   640                                               names[TRY_CATCH]);
>   641         }
>
>
> You could create the form without the identity transform for the non-primitive case?
>
>    final int UNBOX_RESULT = type.returnType().isPrimitive() ? nameCursor++ : 0;
> ...
>
>    if (UNBOX_RESULT > 0) { ...
>        names[UNBOX_RESULT] = new Name(ValueConversions.unbox(type.returnType()), names[TRY_CATCH]);
>    }
I can, but it complicates matching and compiling the pattern in 
InvokerBytecodeGenerator. I decided to keep the shape uniform for all cases.

>
>
>   699         try {
>   700             GUARD_WITH_CATCH
>   701                     = IMPL_LOOKUP.findStatic(MethodHandleImpl.class, "guardWithCatch",
>   702                     MethodType.methodType(Object.class, MethodHandle.class, Class.class, MethodHandle.class, Object[].class));
>   703         } catch (ReflectiveOperationException ex) {
>   704             throw new RuntimeException(ex);
>   705         }
>   706         return GUARD_WITH_CATCH;
>
>
> Should #704 be:
>
>    throw newInternalError(ex);
>
> ?
Fixed. Also updated other places where RuntimeException was thrown.

>
>
> test/java/lang/invoke/MethodHandles/TestCatchException.java
>
>   100         Object o = new Object();
>   101         Object[] obj1 = new Object[] { "str" };
>   102
>   103         Object r1 = target.invokeExact(o, o, o, o, o, o, o, o, obj1);
>   104         Object r2 = gwc.invokeExact(o, o, o, o, o, o, o, o, obj1);
>   105         assertTrue(r1 != null);
>   106         assertTrue(r1.equals(r2));
>
> To be on the safe side should probably assert as follows:
>
>    assertEquals(r1, obj);
>    assertEquals(r2, obj);
Fixed.

Best regards,
Vladimir Ivanov

> Paul.
>
>
> On Feb 19, 2014, at 10:46 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>
>> http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.00
>> https://jbs.oracle.com/bugs/browse/JDK-8027827
>> 354 lines changed: 193 ins; 91 del; 70 mod
>>
>> OVERVIEW
>>
>> MethodHandles.catchException combinator implementation is based on generic invokers (MethodHandleImpl$GuardWithCatch.invoke_*). It is significantly slower than a Java equivalent (try-catch).
>>
>> Future Nashorn performance improvements require catchException combinator speed to be on par with try-catch in Java.
>>
>> So, it should be represented in a more efficient form.
>>
>> I chose the following lambda form representation:
>>
>>   t_{i}:L=ValueConversions.array(a1:L,...,a_{k}:L);
>> t_{i+1}:L=MethodHandleImpl.guardWithCatch(t_{p1}, t_{p2}, t_{p3}, t_{i}:L);
>> t_{i+2}:I=ValueConversions.unbox(t7:L);
>>      OR :L=ValueConversions.identity(t_{n+1})
>>      OR :V=ValueConversions.ignore(t_{n+1})
>>
>> where:
>>   a1, ..., a_{k} - arguments
>>   t_{p1}, t_{p2}, t_{p3} - target method handle, exception class, catcher method handle respectively; passed as bounded parameters;
>>
>> During lambda form compilation it is converted into bytecode equivalent of the following Java code:
>>   try {
>>       return target.invokeBasic(...);
>>   } catch(Throwable e) {
>>       if (!exClass.isInstance(e)) throw e;
>>       return catcher.invokeBasic(e, ...);
>>   }
>>
>> There's a set of microbenchmarks (attached to the bug) I wrote to verify performance characteristics of new implementation.
>>
>> FURTHER WORK
>>
>> What is missing is lambda form caching. The plan is to have a single lambda form per basic type, but it needs more work - current representation is suitable for sharing on bytecode level, but lambda form interpretation doesn't work well (arguments boxing + result unboxing are problematic).
>>
>> TESTING
>>
>> Tests: microbenchmarks, jdk/java/lang/invoke/, nashorn with optimistic types (unit tests, octane).
>>
>> Tested in 2 modes:
>>   * COMPILE_THRESHOLD=30
>>   * COMPILE_THRESHOLD=0 -Xverify:all
>>
>> OTHER
>>
>> 1) Update of cached name and member in LF compilation loop (see InvokerBytecodeGenerator.generateCustomizedCodeBytes) fixes a bug during compilation of selectAlternative when running with COMPILE_THRESHOLD=0.
>>
>> 2) As part of this change, I fix existing bug [1], so I add regression test for it.
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8034120
>