RFR (M): JDK-8038587: [TESTBUG] Create CDS tests to exercise region sizes and classlist

[Mikhailo Seledtsov]

Here is the updated webrev, please review: 
http://cr.openjdk.java.net/~mseledtsov/8038587/webrev.02/

Testing: running these tests via JPRT (jprt-sfbay submit -stree . 
-rtests '*-*-*-runtime/SharedArchiveFile/' -otests '.*runtime.*' -noqa 
-excludetargets 'linux_ppc.*' -excludetargets 'linux_arm.*' -email 
mikhailo.seledtsov at oracle.com)

Misha

On 5/20/2014 5:56 PM, Ioi Lam wrote:
> Misha, sounds good to me.
>
> Thanks
> - Ioi
>
> On 5/20/14, 2:50 PM, Mikhailo Seledtsov wrote:
>> Hi Ioi,
>>
>>  SpaceUtilizationCheck:
>>   I have just discussed this with Coleen. She recommends the following:
>> 1. Only check the space utilization for RO and RW regions, since they 
>> account for most of the reserved space
>> 2. She thinks that 50% minimum utilization is reasonable, for the 
>> bootstrap CDS, for both 32-bit and 64-bit platforms. I agree
>> 3. On 64-bit platforms, better utilization will reduce a chance of 
>> reservation denial due to ASLR. In other words, the smaller space the 
>> CDS is asking for to reserve, the smaller chance of collision with an 
>> already-mapped region.
>>
>> I am going to update the code accordingly. Please let me know if you 
>> have any objections to this approach.
>>
>> Misha
>>
>> On 5/20/2014 3:19 PM, Mikhailo Seledtsov wrote:
>>> Hi Ioi,
>>>
>>>  Thank you for reviewing the tests. Please see my comments inline, 
>>> let me know if you disagree.
>>> I will re-work the code based on these comments, and post the 
>>> updated webrev.
>>>
>>> Misha
>>>
>>> On 5/19/2014 4:45 PM, Ioi Lam wrote:
>>>> Hi Misha,
>>>>
>>>> SharedBaseAddress.java:
>>>>
>>>>    During dumping, the VM will first try to map at the address
>>>>    specified by -XX:SharedBaseAddress. However, if this fails (another
>>>>    mapping already exists there, the VM will simply map a random
>>>>    address (as selected by the OS).
>>>>
>>>>    I am not sure if there's anything you need to check here, but 
>>>> just FYI.
>>>>
>>> The main goal of this test is too make sure that JVM does not crash 
>>> or throw exception, no additional checks.
>>>> SpaceUtilizationCheck:
>>>>
>>>>    It will probably never happen, but in the rare case you may have
>>>>    100% utilization. So for robustness, you may want to handle this in
>>>>
>>>>        result.add(input.substring(m.start() + 1, m.start() + 3 ));
>>> Good catch. Thank you. I will add this case to the matcher logic, as 
>>> well as a single-digit utilization case (e.g. 3.3%)
>>>>
>>>>    Also, 75% may be a good value for 32-bit (where available address
>>>>    space is scarce), but for 64-bit, I think it's OK to have a low
>>>>    utilization (i.e., a large default size) so that it's easier for 
>>>> the
>>>>    user to try different class list without having to worry about the
>>>>    region sizes.
>>>>
>>> I agree, thank you for pointing this out. I will add a check to have 
>>> different thresholds for 32-bit vs 64-bit. I still think that 64-bit 
>>> should have some reasonable minimum value for utilization.
>>> How about 30%?
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>>
>>>>
>>>>
>>>> On 5/15/14, 4:15 PM, Mikhailo Seledtsov wrote:
>>>>> Hi David, team,
>>>>>
>>>>>  After  more discussions on the usefulness and stability of the 
>>>>> ClassListExerciser test with the team, we have decided that this 
>>>>> test is not that useful. Thank you David for your comments again.
>>>>> I have kept two other tests, and added a new test: 
>>>>> SharedBaseAddress.java, which was in the plans and is intended to 
>>>>> exercise various values for the SharedBaseAddress CL flag.
>>>>>
>>>>> The updated webrev can be found at: 
>>>>> http://cr.openjdk.java.net/~mseledtsov/8038587/webrev.01/
>>>>> The bug name has been changed to: [TESTBUG] Create CDS tests to 
>>>>> exercise region sizes and base address
>>>>>
>>>>> Thank you,
>>>>> Misha
>>>>>
>>>>> On 4/2/2014 7:55 PM, Mikhailo Seledtsov wrote:
>>>>>> David,
>>>>>>
>>>>>>  Thank you. I will rework ClassListExerciser test to take your 
>>>>>> comments into consideration, and will submit a new webrev.
>>>>>>
>>>>>> Misha
>>>>>>
>>>>>> On 4/1/2014 9:52 PM, David Holmes wrote:
>>>>>>> On 2/04/2014 7:06 AM, Mikhailo Seledtsov wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>>   Thank you for review and your feedback.
>>>>>>>>
>>>>>>>> The intent of this test is sanity check of basic functionality, 
>>>>>>>> making
>>>>>>>> sure the shared classes are loaded w/o crashes or errors. Even 
>>>>>>>> though
>>>>>>>> creating a shared archive with -Xshare:dump does exercise 
>>>>>>>> loading of the
>>>>>>>> classes from the classlist, I believe SQE should verify it, by
>>>>>>>> explicitly performing this operation. In my experience I have 
>>>>>>>> found that
>>>>>>>> basic tests often find interesting bugs.
>>>>>>>>
>>>>>>>> I did drop the attempt to instantiate classes, because the 
>>>>>>>> amount of
>>>>>>>> classes in the class list that have default constructors and 
>>>>>>>> instantiate
>>>>>>>> successfully is quite small, and not worth the trouble. Many 
>>>>>>>> classes
>>>>>>>> fail instantiation due to the absence of UI, or other valid 
>>>>>>>> reasons.
>>>>>>>
>>>>>>> Okay. Dropping that seems to alleviate most of my concerns.
>>>>>>>
>>>>>>>> What I have found, however, as part of this exercise, is that the
>>>>>>>> default SE classlist is optimized for the client, not the server.
>>>>>>>>
>>>>>>>> As for classes that are part of the classlist, but are really 
>>>>>>>> missing
>>>>>>>> from rt.jar: will you consider this to be a bug?
>>>>>>>
>>>>>>> No. The default classlist, as you note is defined for a 
>>>>>>> particular scenario - at the moment "client" apps. But many of 
>>>>>>> those classes are not present in Compact Profiles. So 
>>>>>>> unless/until we have customized default classlists for Compact 
>>>>>>> Profiles, missing classes can be expected. I don't see this as 
>>>>>>> an issue that warrants such customized classlists.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>> Misha
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/1/2014 1:46 AM, David Holmes wrote:
>>>>>>>>> Hi Misha,
>>>>>>>>>
>>>>>>>>> On 28/03/2014 5:34 AM, Mikhailo Seledtsov wrote:
>>>>>>>>>> Please review these 3 new CDS tests, an ongoing effort in 
>>>>>>>>>> implementation
>>>>>>>>>> of the CDS test specification.
>>>>>>>>>>
>>>>>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8038587
>>>>>>>>>>      Webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~mseledtsov/8038587/webrev.00/
>>>>>>>>>>      Testing:
>>>>>>>>>>          Local testing on multiple platforms
>>>>>>>>>>          JPRT to exercise the added tests:
>>>>>>>>>> 2014-03-27-184953.mseledtsov.cds (PASS)
>>>>>>>>>>          These tests found 2 bugs, and one potential issue
>>>>>>>>>
>>>>>>>>> I don't quite get the point of the ClassListExerciser test. The
>>>>>>>>> classlist may well contain classes that do not exist, or that 
>>>>>>>>> can not
>>>>>>>>> be instantiated in the test context, even if they have a no-arg
>>>>>>>>> constructor. Simply creating an archive "exercises" the 
>>>>>>>>> classlist, so
>>>>>>>>> I'm really not sure what this test is intending to test.
>>>>>>>>>
>>>>>>>>> Also this test won't work with SE Embedded as we have a 
>>>>>>>>> customized
>>>>>>>>> default classlist for the Embedded stack.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>> Misha
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>