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 <[email protected]>
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
