John,
Thanks for review!
Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.02/
See my comments inline.
Also, integrated some pending cleanups (e.g. improved selectAlternative
detection).
Best regards,
Vladimir Ivanov
On 2/24/14 9:46 PM, John Rose wrote:
On Feb 20, 2014, at 9:57 AM, Vladimir Ivanov
<vladimir.x.iva...@oracle.com <mailto:vladimir.x.iva...@oracle.com>> wrote:
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.
This is a good step forward, thanks!
Some comments:
I prefer the bounds check expression pos+3 < lambdaForm.names.length.
(One integer constant, limit to right.)
Fixed.
The predicate isGuardWithCatch must test all three subforms. Or else
there must be assertions to ensure that names[pos+2] is of the expected
form. The problem is that LF's can sometimes be edited (e.g., by
binding operations) and there is no insurance that your pattern of three
expressions will be preserved in all cases.
Fixed.
I see you are trying to do unboxing elimination here; this is not a safe
or effective way to do it, IMO. Put in a "FIXME" comment and file a bug
to deal better with unboxing ops in LFs. I have some WIP code toward
this end which we can talk about. You've probably seen of that
business, about internally LF marking expressions as intrinsics to guide
bytecode generation.
Added a comment. Will file a bug once this change is in.
Why is the logic about cachedLambdaForm commented out? It looks
correct, but is there a bug?
I think you looked at a stale version. Latest webrev have caching enabled.
I had some problems with sharing before because boxing/unboxing is
specific for different types and couldn't be shared among combinators
with the same basic type.
Consider replacing GUARD_WITH_CATCH with Lazy.NF_guardWithCatch, and
using the NF instead of MH for the intrinsic.
Done. Also updated other similar places in MethodHandleInfo.
— John
