RFR (S) 8023697: failed class resolution reports different class name in detail message for the ,first and subsequent times

[Coleen Phillimore]

Karen,

Thank you for the thorough code review and questions about footprint.  
I'm glad we verified this!

On 5/1/14, 1:06 PM, Karen Kinnear wrote:
> Coleen,
>
> Code looks good and so much cleaner!
>
> couple of minor comments - and you can check this in without my needing to re-review:
>
> 1) Do you mind putting in an assertion in save_and_throw_exception that you are holding the cp_lock()?

Ok. No problem.  I added it.

> 2) resolutionErrors.cpp free_entry
>     - is there a place where you free the message if it is not null, and decrement the refcount for it as well?

I missed that.  Thank you!
>
> Thank you for the refactoring and cleanups while you were testing this area.
> I like the class names in your test :-)
>
> Thank you for the extra research to allay my concerns about footprint increase if we saved a lot of long error messages.
> I really appreciate the extra investigation.

Thank you for suggesting and the careful code review.

Coleen
>
> thanks,
> Karen
>
> On Apr 30, 2014, at 5:41 PM, Coleen Phillimore wrote:
>
>> Karen,  Thank you for looking at the code.
>>
>> On 4/28/14, 6:11 PM, Karen Kinnear wrote:
>>> Coleen,
>>>
>>> A couple of comments/questions:
>>>
>>> 1. Can you possibly measure footprint in a large application to see if the saved error messages use significant footprint?
>> I ran medrec and dacapo.  Only failed constant pool resolutions are cached in this resolution error table.  For these applications there was only one entry in this hashtable, so the footprint increase for both the pointer and the refcounted Symbol (which is the class name) is mimimal.   The spec requires that the JVM throw the same exception for subsequent resolutions from the constant pool.  For the field and method cases, we repeat the resolution steps and get the same answer.   Only for classes, method handles and method types, do we save the resolution error.  The resolution error is for errors during class loading from constant pool symbolic reference in the spec 5.3 paragraph 1 and does not include errors for linking and initializing the class.  The error conditions for this are saved with a different mechanism (InstanceKlass::is_in_error_state()).
>>
>>> 2. I'm concerned about setting expectations that the detailed message will be repeated precisely for constant pool resolution
>>>      - but for now that is only for Class, MethodHandle and MethodType resolution
>> Since the JVM repeats the attempted resolution for these other types of constant pool resolution, the message will already be the same.
>>
>>> 3. A comment in the bug report says that for linkage errors in SD the detail message is the name of the class, in all cases but this one, the
>>> message is the same
>>>     - I'm confused - did you test this with for instance a LoaderConstraint test - doesn't that have a different detail message?
>> I fixed the comment in the bug.  For the case in the test, where the error came from the super class not found, the message refers to the super class.  For all the other testing I did, since we save the name of the class in the error message.  The loader constraint test does not save the error in the resolution error table.
>>
>> In investigating this, I found there were two  parameters unused in handle_resolution_exception() so cleaned them up too.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8023697_2/
>>
>> Thanks,
>> Coleen
>>> I appreciate consolidating the duplicate code.
>>>
>>> thanks,
>>> Karen
>>>
>>> On Apr 28, 2014, at 5:07 PM, Coleen Phillimore wrote:
>>>
>>>> Summary: Cache detail message when we cache exception for constant pool resolution.
>>>>
>>>> In the resolution error table.  Provide a default message if one is not in the original exception.  Also, consolidate duplicate code for MethodHandleInError and MethodTypeInError with UnresolvedClassInError.
>>>>
>>>> open webrev athttp://cr.openjdk.java.net/~coleenp/8023697/
>>>> bug linkhttps://bugs.openjdk.java.net/browse/JDK-8023697
>>>>
>>>> Ran jck8, ute vm.quick.testlist, jtreg tests in hotspot/test, java/lang/invoke jdk jtreg tests.
>>>>
>>>> Thanks,
>>>> Coleen