RFR 8016325: JVM hangs verifying system dictionary

[Coleen Phillimore]

Thanks Jon for looking at this.

On 06/24/2013 02:21 PM, Jon Masamitsu wrote:
> Coleen,
>
> http://cr.openjdk.java.net/~coleenp/8016325/src/share/vm/oops/klass.cpp.frames.html 
>
>
> 650 void Klass::verify_on(outputStream* st, bool in_dictionary) {
>
>
> "in_dictionary" is not used, right?

It isn't used but I forgot to change the name of the parameter to 
check_dictionary which has the opposite sense.
>
> http://cr.openjdk.java.net/~coleenp/8016325/src/share/vm/oops/klass.hpp.frames.html 
>
>
> So do you need this change to add the parameter "check_dictionary"?
>>   706   virtual void verify_on(outputStream* st, bool 
>> check_dictionary = true);
>>   707   void verify(bool check_dictionary = true) { verify_on(tty, 
>> check_dictionary); }
>>   708
> If you do, is the "virtual" on 706 needed?  And are the number of 
> lines you would
> have to change if "check_dictionary" does not have a default  (not a 
> fan of defaults),
> so large?

Yes, it's needed because then InstanceKlass::verify_on will override 
this versions.  It won't if the parameters don't match.   We definitely 
want verify_on() to be virtual.

I'm afraid the number of lines would be pretty large if there wasn't a 
default parameter.

>
> What's the rule about when to use is_metaspace_object() to
> do an assertion check?

The rule I applied was to use this test if the metadata object was 
coming from some location where non-metadata could also be stored by 
mistake (like the frame_x86.hpp example).   In the verification code the 
compiler's static type checking code can do the job for us so that's why 
I removed them.

Thanks,
Coleen

>
> Rest looks good.
>
> Jon
>
>
> On 6/21/13 9:35 AM, Coleen Phillimore wrote:
>> Summary: Minimize redundant verifications of Klasses.
>>
>> Also removed is_metadata() because checking that metadata is not in 
>> the Java heap doesn't make sense anymore and could slow down 
>> verification also.   Some cases that aren't statically checked by the 
>> compiler now call is_metaspace_object() under assert for verification.
>>
>> Tested by nsk.quick.testlist, and failing SQE test.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8016325/
>> bug link at http://bugs.sun.com/view_bug.do?bug_id=8016325
>> local bug link https://jbs.oracle.com/bugs/browse/JDK-8016325
>>
>> thanks,
>> Coleen
>