Code and Design Feedback request: 7114376: tune system dictionary size

Karen Kinnear karen.kinnear at oracle.com
Mon Jan 30 17:07:07 PST 2012


Thanks David. The amount of benchmarking for using this new flag has been more limited than
I would like for a fully supported flag. And given that we are going to be revising the underlying
data structure later, it may be more appropriate to offer a different flag, so this sets different
expectations on whether this flag will still be supported.

thanks for all the reviews,
Karen

On Jan 30, 2012, at 7:39 PM, David Holmes wrote:

> Hi Karen,
> 
> Other than that I don't see why this needs to be experimental I don't have any further comments.
> 
> Thanks,
> David
> 
> On 31/01/2012 4:35 AM, Karen Kinnear wrote:
>> David et al,
>> 
>> Latest webrev:
>> http://cr.openjdk.java.net/~acorn/7114376.03/webrev/
>> 
>> Good catch. I put this back - the shared archive system dictionary size doesn't change.
>> 
>> Paul also asked for #defines to be moved into the Constants enum, so I did that as well.
>> And I made this experimental, so it requires the -XX:+UnlockExperimentalVMOptions.
>> 
>> thanks,
>> Karen
>> 
>> p.s. again: manually tested. JPRT in progress, nsk's resubmitted.
>> 
>> On Jan 30, 2012, at 12:04 AM, David Holmes wrote:
>> 
>>> Hi Karen,
>>> 
>>> On 30/01/2012 1:12 PM, Karen Kinnear wrote:
>>>> Final review please -
>>>> 
>>>> I reduced this to just add the command-line flag -XX:PredictedLoadedClassCount=#
>>>> and left the dynamic resizing for later.
>>> 
>>> Ok.
>>> 
>>> systemDictionary.cpp:
>>> 
>>> 1171 void SystemDictionary::set_shared_dictionary(HashtableBucket* t, int length,
>>> 1172                                              int number_of_entries) {
>>> 1173   int newsize = length / sizeof(HashtableBucket);
>>> 1174   _shared_dictionary = new Dictionary(_nof_buckets, t, number_of_entries);
>>> 1175 }
>>> 
>>> newSize is calculated but not used.
>>> 
>>> David
>>> -----
>>> 
>>> 
>>>> webrev: http://cr.openjdk.java.net/~acorn/7114376.02/webrev/
>>>> 
>>>> Testing:
>>>> Manual testing of command-line options, CDS, scimark.
>>>> In parallel: JPRT, nsk, benchmarks
>>>> 
>>>> thanks,
>>>> Karen
>>>> 
>>>> p.s. the following files did not change between webrev.01 and webrev.02
>>>> agent/...
>>>> globals.hpp
>>>> vmStructs.cpp
>>>> 
>>>> On Jan 27, 2012, at 6:13 PM, Karen Kinnear wrote:
>>>> 
>>>>> Dan&   David -
>>>>> 
>>>>> Thank you both for the detailed reviews.
>>>>> 
>>>>> I will make the changes suggested - but I will put the code changes on hold. I think
>>>>> the best approach right now is to just add the command-line flag and then after
>>>>> we get the permanent generation changes, rethink the loaded class cache storage
>>>>> options - and do a whole lot more measurements than I am able to do in this timeframe -
>>>>> even thanks to the help I have had.
>>>>> 
>>>>> Details below,
>>>>> thanks,
>>>>> Karen
>>>> 
>> 



More information about the hotspot-dev mailing list