Claes, Discussed with Lois. We think that it would make more sense to pass the new argument into MethodHandles::resolve_MemberName and at all three places that we currently CHECK_PENDING_EXCEPTION/return null there - if speculative flag is set - CLEAR_PENDING_EXCEPTION before you return null - and yes - do this for all three cases, not just the METHOD case
Couple of questions though: 1) do you actually reach the new code in MHN_resolve_Mem? Could you possibly add an assertion there that there is no exception pending and/or print the exception if there is one pending? With the CHECK_NULL in the call to resolve_MemberName I do not expect you to get here with a pending exception, so the question arises as to when you would have a null resolved, but no pending exception? 2) with the change you just sent out - do you really get a performance improvement? I’m confused about where that comes from My apologies - I was incorrect - we are not converting Error -> Exception in MHN_resolve_Mem - so I do not know why we are burying existing exceptions here - longer-term I think we need to clean this up and as David points out - at least for the ResolveOrFail case - store the original exception as a cause. thanks, Karen > On Mar 26, 2018, at 8:08 AM, Claes Redestad <claes.redes...@oracle.com> wrote: > > Hi, > > MethodHandleNatives::resolve are used by both > MemberName$Factory::resolveOrFail and ::resolveOrNull, the latter allowing > speculative lookup of methods. > > While the linkResolver code this calls into in the VM will create and throw > exceptions for all cases where a method or field is missing, these are caught > by MethodHandles::resolve_MemberName and an empty handle is returned instead. > However, the outer method, MHN_resolve_Mem will always create a new exception > and throw it. > > In case of resolveOrNull, we'll then ignore whatever exception is thrown in > the java code, so in effect we're creating and ignoring two exceptions on a > failed lookup. > > We can cut this in half by passing a boolean to MethodHandleNatives::resolve > to indicate whether we're doing a speculative lookup (resolveOrNull) or not > (resolveOrFail): > > Bug: https://bugs.openjdk.java.net/browse/JDK-8200238 > > Webrev: http://cr.openjdk.java.net/~redestad/8200238/open.00/ > > This is a small startup optimization for applications that have a high miss > rate (looks up a lot of methods that doesn't exist in the pre-generated > Holders). > > Thanks! > > /Claes >
