RFR (S): 8208061: runtime/LoadClass/TestResize.java fails with "Load factor too high" when running in CDS mode

[Ioi Lam]

Hi Jiangli,

The updated webrev looks good.


On 8/24/18 10:34 AM, Jiangli Zhou wrote:
> Hi Ioi,
>
> Thanks for the review and suggestions. Here is the updated webrev: 
> http://cr.openjdk.java.net/~jiangli/8208061/webrev.01/. Please see 
> comments below.
>
>
> On 8/23/18 9:13 PM, Ioi Lam wrote:
>> Hi Jiangli,
>>
>> The changes look good. I have a couple of suggestions:
>>
>>
>> classLoaderData.cpp:
>>
>>   if (!DynamicallyResizeSystemDictionaries || DumpSharedSpaces) {
>>
>> I think we can also remove the DumpSharedSpaces condition. As far as 
>> I know, during dump time, we use the ClassLoaderData API to access 
>> the dictionaries, so even if the dictionary is resized we should be OK.
>
> I was considering that also but decided to delay it. There might be 
> unexpected issues when we start doing dynamic dumping. I think it's 
> probably better to address this together with dynamic dumping. I will 
> file a RFE so we don't forget this. Thoughts?

Regarding dynamic dumping, I think it's going to be drastically 
different than how we dump the archive today. Specifically, we cannot 
rely on the DumpSharedSpaces flag, which currently makes the VM behave 
very differently than a "normal" run. So, if we want to dump the archive 
while doing a normal run, most likely we won't turn on the 
DumpSharedSpaces flag, or we might have to significantly change how 
DumpSharedSpaces works.

So I think it's better to remove DumpSharedSpaces from this line. It's 
OK to do it in an RFE if you don't want to spend time testing for it 
right now.

Thanks
- Ioi


>>
>> systemDictionary.cpp
>>
>> SystemDictionary::set_shared_dictionary:
>>
>> I think we should add an assert:
>>
>>   assert(!DumpSharedSpaces, "Should not be called with 
>> DumpSharedSpaces");
> Added assert.
>
> Thanks,
> Jiangli
>>
>>
>> Thanks
>> - Ioi
>>
>>
>> On 8/23/18 5:40 PM, Jiangli Zhou wrote:
>>> Please review the bug fix for JDK-8208061. The 
>>> runtime/LoadClass/TestResize.java test failed with "Load factor too 
>>> high" when running with CDS enabled at runtime. Dictionary resizing 
>>> was disable when UseSharedSpaces was true, which caused the failure.
>>>
>>> The fix is to allow resizing for all system dictionaries except for 
>>> the shared dictionary at runtime. The shared dictionary data are 
>>> from mapped CDS archive and does not change at runtime, so resizing 
>>> is not required. The change also adds 'resizable' information to the 
>>> log output in Dictionary::print_on().
>>>
>>> webrev: http://cr.openjdk.java.net/~jiangli/8208061/webrev.00
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8208061
>>>
>>> Verified with TestResize.java running locally with CDS enabled. 
>>> Tier1 - tier5 tests are in progress.
>>>
>>> Thanks,
>>>
>>> Jiangli
>>>
>>
>