Fwd: Re: RFR: JDK-8072725: Provide more granular levels for GC verification

Poonam Bajaj Parhar poonam.bajaj at oracle.com
Thu Jan 7 18:30:49 UTC 2016


Hello Kevin,

On 1/7/2016 8:28 AM, Kevin Walls wrote:
>
> Thanks Poonam, that looks good to me. 8-)
>
> OK, maybe we abort somehow if os::malloc returns null, and maybe:
>
> tty->print_cr("VerifySubSet:%s  is unknown, skipping it.", token);
> should be:
> tty->print_cr("VerifySubSet: '%s' is unknown, skipping it.", token);
>
Fixed it:
http://cr.openjdk.java.net/~poonam/8072725/webrev.02/

> ..but nothing more from me!

Thanks,
Poonam

>
> Thanks
> Kevin
>
>
> On 07/01/2016 14:28, Poonam Bajaj Parhar wrote:
>> Hello Kevin,
>>
>> Thanks for looking at the changes and for your feedback.
>>
>> On 1/5/2016 1:08 PM, Kevin Walls wrote:
>>> Hi Poonam,
>>>
>>> Great, I think it looks good, and really look forward to having this 
>>> ability to fine-tune what we verify, rather than being so 
>>> heavyweight about it!
>>>
>>> I just noticed: should_verify_subset("heap") and 
>>> should_verify_subset("c-heap") are going to clash?  If you specify 
>>> c-heap, you'll trigger both verifications as strstr finds "heap" in 
>>> there?  Do you want to change heap to java-heap, or maybe use 
>>> java_heap and c_heap (if you want to use underscores to be 
>>> consistent with symbol_table, string_table), or something else to 
>>> make them not clash?
>>>
>> Yes, it is a problem with the current code. To avoid this problem and 
>> such future problems when we may add more subsets, I have updated the 
>> code to parse the SubSet string once and store the choices in a flag. 
>> And when we need to verify, we check for the bits in this flag.
>>
>>>
>>> (As we discussed over email briefly, another possibility was 
>>> splitting the given argument string on commas, doing our string 
>>> comparisons once only during argument processing somewhere, and 
>>> populating a long of flag bits, so Universe::verify() just checks 
>>> for presence of bits, rather than comparing lots of characters.  The 
>>> speed isn't a particular worry right now so your very readable 
>>> string comparisons do the job!)
>>
>> Here's the updated webrev:
>> http://cr.openjdk.java.net/~poonam/8072725/webrev.02/
>>
>> regards,
>> Poonam
>>
>>>
>>> Thanks
>>> Kevin
>>>
>>>
>>>>
>>>> -------- Original Message --------
>>>> Subject: 	Re: RFR: JDK-8072725: Provide more granular levels for GC 
>>>> verification
>>>> Date: 	Wed, 23 Dec 2015 09:53:21 -0800
>>>> From: 	Jon Masamitsu <jon.masamitsu at oracle.com>
>>>> Organization: 	Oracle Corporation
>>>> To: 	hotspot-gc-dev at openjdk.java.net
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 12/23/2015 9:26 AM, Poonam Bajaj Parhar wrote:
>>>>> Hello Jon,
>>>>>
>>>>> I have added a testcase to test the VerifySubSet option:
>>>>> http://cr.openjdk.java.net/~poonam/8072725/webrev.01/
>>>>>
>>>>> Here, I run the test with one set of VerifySubSet options, and 
>>>>> then check the output that those sub-systems were verified and the 
>>>>> ones not specified were not verified.
>>>>
>>>> Excellent.  Thanks for adding the test.
>>>>
>>>> Reviewed.
>>>>
>>>> Jon
>>>>
>>>>>
>>>>> Thanks,
>>>>> Poonam
>>>>>
>>>>> On 12/22/2015 7:43 AM, Jon Masamitsu wrote:
>>>>>>
>>>>>>
>>>>>> On 12/21/2015 4:40 PM, Poonam Bajaj Parhar wrote:
>>>>>>> Hello Jon,
>>>>>>>
>>>>>>> On 12/21/2015 2:29 PM, Jon Masamitsu wrote:
>>>>>>>> Poonam,
>>>>>>>>
>>>>>>>> Some of the string list parameters allow separation by , or " 
>>>>>>>> ".  Did
>>>>>>>> you consider adding that?
>>>>>>>>
>>>>>>> The VerifySubSet string list can accept the strings separated by 
>>>>>>> comma or a space. Before verification, we just need to check if 
>>>>>>> a particular subset is present in the VerifySubSet list or not. 
>>>>>>> We don't need any complicated processing of the strings list here.
>>>>>>
>>>>>> Ok.  Thanks for the explanation.
>>>>>>
>>>>>> You mention in the RFR that you have a simple test to test the
>>>>>> change.  Can you add that as a jtreg test?  I'm thinking about
>>>>>> something like
>>>>>>
>>>>>> test/gc/TestVerifyDuringStartup.java
>>>>>> test/gc/TestVerifySilently.java
>>>>>>
>>>>>> Jon
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Poonam
>>>>>>>
>>>>>>>> Jon
>>>>>>>>
>>>>>>>> PS. See share/vm/compiler/compilerDirectives.cpp
>>>>>>>>
>>>>>>>> ccstrlist DirectiveSet::canonicalize_disableintrinsic(ccstrlist 
>>>>>>>> option_value)
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/15/2015 03:15 PM, Poonam Bajaj Parhar wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Please review these changes that split up the work done under 
>>>>>>>>> Verify*GC options. This will be very useful in debugging GC 
>>>>>>>>> issues/crashes where verifying the whole memory system with 
>>>>>>>>> Verify*GC options slows down the process and makes it 
>>>>>>>>> impossible to reproduce the problem.
>>>>>>>>>
>>>>>>>>> The changes introduce a new option /VerifySubSet///that can be 
>>>>>>>>> used to specify the specific memory sub-systems that one wants 
>>>>>>>>> to verify. It can be one or more of the sub-systems from 
>>>>>>>>> these: threads, heap, symbol_table, string_table, codecache, 
>>>>>>>>> dictionary, classloader_data_graph, metaspace, jni_handles, 
>>>>>>>>> c-heap and codecache_oops. If nothing is specified with 
>>>>>>>>> /VerifySubSet/ then whole of the memory system is verified 
>>>>>>>>> with Verify*GC options.
>>>>>>>>>
>>>>>>>>> Bug:JDK-8072725: 
>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8072725> Provide 
>>>>>>>>> more granular levels for GC verification
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~poonam/8072725/webrev.00/
>>>>>>>>> Testing: JPRT, tested the changes with a simple test program.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Poonam
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160107/56eaff16/attachment.htm>


More information about the hotspot-gc-dev mailing list