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

[Kevin Walls]

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);

..but nothing more from me!

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/f8189f38/attachment.htm>