RFR: 8184765: Dynamically resize SystemDictionary

[coleen.phillimore at oracle.com]

On 10/13/17 9:11 AM, David Holmes wrote:
> Hi Karen,
>
> On 13/10/2017 8:42 PM, Karen Kinnear wrote:
>> A quick note - we are resizing the hash table, i.e. number of linked 
>> lists, so as to speed up lookup if it is too small, or waste less 
>> space if it is
>> too large. Each dictionary may have multiple entries off of it - one 
>> per loaded class - so there is no such thing as running out
>> of space based on the sizing (just based on total OOM). I have not 
>> yet looked at the change - but just wanted to clarify the
>> goal and concept for folks first.
>
> Thanks for clarifying - though AFAICS this proposal only grows the 
> dictionary. Is the "waste less space" to be achieved by starting with 
> a smaller dictionary initially? Do we have a way to control the 
> initial size?

I made the change so that there is one dictionary per class loader 
data.  The <bootloader> dictionary starts at 1009 and from what I've 
seen in testing has about 900 entries in it on average. So seems 
appropriately sized.  The application class loader dictionary (as 
default) or dictionary defined with -Djava.system.class.loader and the 
other dictionaries start at 107.   The latter can be started with a 
larger size with -XX:+PredictedLoadedClassCount.   We had trouble 
finding any users who set this option.

The application class loader dictionary is hard to find an initial size 
for, and rather than have the user guess which size they want, we will 
do automatic resizing to make it larger.  I think the initial size of 
application class loader dictionary should be more like 1009.  It was a 
bug that I initialized it with 107 actually.

We do not have a way to specify initial size for class loader 
dictionaries.   And we don't resize down because that doesn't make any 
sense.   Entries are almost never deleted(*), rather the whole 
dictionary goes away when the class is unloaded.

So this change is mostly for a class loader that loads a huge number of 
classes, that might experience slower than average lookups.  We do have 
a user who does this internally.

Coleen

> Thanks,
> David
>
>> thanks
>> Karen
>>
>>> On Oct 11, 2017, at 4:10 PM, coleen.phillimore at oracle.com wrote:
>>>
>>>
>>> Gerard, some preliminary comments.
>>>
>>> http://cr.openjdk.java.net/~gziemski/8184765_rev1/src/hotspot/share/classfile/systemDictionary.cpp.udiff.html 
>>>
>>>
>>> *+{*
>>> *!_MutexLocker mu(SystemDictionary_lock, THREAD_);*
>>> *!_Klass* probe = dictionary->find(_d_hash, name, protection_domain);*
>>> *if (probe != NULL) return probe;*
>>> **
>>> *+ }*
>>>
>>> I don't think you need this because dictionary->find() should have 
>>> the NoSafepointVerifier, so the index will not change.
>>>
>>> http://cr.openjdk.java.net/~gziemski/8184765_rev1/src/hotspot/share/classfile/classLoaderData.cpp.udiff.html 
>>>
>>>
>>> I think we want a global _some_dictionary_needs_resizing to avoid 
>>> walking through the CLDG.
>>>
>>> And have Dictionary have a field _resizing_needed to avoid the 
>>> calculation during the safepoint, which is set when adding an entry.
>>>
>>> _resizing_needed = *(number_of_entries() > 
>>> (_resize_load_trigger*table_size());
>>> *CLDG::_any_resizing_needed |= _resizing_needed;   // or something 
>>> like that.
>>>
>>> I can write more about the rationale of this change in the bug 
>>> report, if needed.
>>>
>>> Thank you for doing this change.
>>> Coleen
>>>
>>>
>>> On 10/10/17 4:40 PM, Gerard Ziemski wrote:
>>>> hi all,
>>>>
>>>> Please review this change that adds dynamic resizing of system 
>>>> dictionaries.
>>>>
>>>> The biggest change is refactoring of the code that protects 
>>>> calculating of the table entry’s index using SystemDictionary_lock
>>>>
>>>> A few notes:
>>>>
>>>> - dynamic resizing is off when either dumping or using shared spaces
>>>> - we remove the experimental runtime option 
>>>> “PredictedLoadedClassCount” and add 
>>>> “DynamicallyResizeSystemDictionaries” to turn the resizing off 
>>>> (it’s on by default)
>>>> - the jtreg test uses stream of bytes to dynamically load numbered 
>>>> classes from memory instead of disk (thank you Ioi)
>>>>
>>>> bug:    https://bugs.openjdk.java.net/browse/JDK-8184765
>>>> webrev: http://cr.openjdk.java.net/~gziemski/8184765_rev1
>>>>
>>>>
>>>> cheers
>>>>
>>>
>>