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

Mikhailo Seledtsov mikhailo.seledtsov at oracle.com
Tue May 20 19:19:59 UTC 2014


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



More information about the hotspot-runtime-dev mailing list