[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR (S): 8208061: runtime/LoadClass/TestResize.java fails with "Load factor too high" when runni
From:       Jiangli Zhou <jiangli.zhou () oracle ! com>
Date:       2018-08-24 19:10:53
Message-ID: a19d8910-156e-b75c-7f24-97989ded476b () oracle ! com
[Download RAW message or body]

Thanks!

Jiangli


On 8/24/18 12:05 PM, Ioi Lam wrote:
> 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
>>>>
>>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]