On Feb 20, 2014, at 9:57 AM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> wrote:
> 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. A tiny bit of history here on newInternalError: in JDK 8 we added new constructors that take a cause as argument; JDK 7 doesn’t have them. In order to make java.lang.invoke backporting from 8 to 7 easier we used a wrapper method to instantiate InternalErrors. > >> >> >> 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.iva...@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 >> > _______________________________________________ > mlvm-dev mailing list > mlvm-dev@openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
