RFR (S) 8213587 - Speed up CDS dump time by using resizable hashtables

[coleen.phillimore at oracle.com]

Hi Ioi,  I really like this new wrapper on the old hashtable to not have 
to write the boilerplate code!

http://cr.openjdk.java.net/~iklam/jdk12/8213587-resize-cds-hashtables.v02/src/hotspot/share/utilities/hashtable.hpp.udiff.html

+ MEMFLAGS F = mtInternal,


I think you should require the mt type though and not make it a default 
parameter. mtInternal is not very useful to finding memory leaks.

Apart from this (which I don't need to see another version), your change 
looks good and nice to get good performance benefits from this.

thanks,
Coleen

On 11/15/18 12:31 AM, Ioi Lam wrote:
> Coleen pointed out to me off-line that the good old (and ugly) 
> BasicHashtable already supports resizing. I think that might be a 
> better starting point for this RFE:
>
> http://cr.openjdk.java.net/~iklam/jdk12/8213587-resize-cds-hashtables.v02/ 
>
>
> I wrote a new template class called "KVHashtable" (copying the style 
> from ResourceHashtable). That way, you can instantiate different (Key 
> -> Value) mappings without writing tons of boilerplate code. The 
> performance is similar to my previous version, and the code is much 
> cleaner.
>
> I also renamed the RFE title, as well as the subject line of this RFR 
> e-mail.
>
> Thanks
>
> - Ioi
>
>
> On 11/14/18 5:14 PM, Jiangli Zhou wrote:
>> Hi Ioi,
>>
>> On 11/14/18 9:09 AM, Ioi Lam wrote:
>>>
>>>
>>> On 11/13/18 4:05 PM, Jiangli Zhou wrote:
>>>> Hi Ioi,
>>>>
>>>> The change looks reasonable to me in general. It would be helpful 
>>>> to see the performance difference with the expendable table. Do you 
>>>> have any data when large number of classes are loaded (>20000)? How 
>>>> much saving does it provide?
>>>>
>>>
>>> Hi Jiangli, thanks for the review. For dumping 30292 classes:
>>>
>>> BEFORE: 93.971 sec
>>> AFTER:  34.761 sec
>>
>> Thanks for the data! That's about 2.6x improvement with large set of 
>> classes.
>>
>> Thanks,
>> Jiangli
>>>
>>> Thanks
>>> - Ioi
>>>
>>>> Thanks,
>>>>
>>>> Jiangli
>>>>
>>>>
>>>> On 11/8/18 10:35 PM, Ioi Lam wrote:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8213587
>>>>> http://cr.openjdk.java.net/~iklam/jdk12/8213587-configurable-resource-hash.v01/ 
>>>>>
>>>>>
>>>>> TL;DR: -- add a subclass to ResourceHashtable to allow the table 
>>>>> size to be
>>>>>           dynamically specified when the table is constructed.
>>>>>
>>>>>
>>>>>         *** C++ template guru alert ***
>>>>>
>>>>>
>>>>> I don't know much about C++ templates, so my attempt on doing this 
>>>>> may be
>>>>> ill-advised.
>>>>>
>>>>> I *think* that with my patch, the performance of existing code, 
>>>>> which uses
>>>>> a statically-defined SIZE,  should not be affected, as the C++ 
>>>>> compiler
>>>>> should be able to constant-propagate and reduce the new code:
>>>>>
>>>>>   ALWAYSINLINE unsigned size() const {
>>>>>     if (SIZE != CONFIGURABLE_SIZE) {
>>>>>       return SIZE;
>>>>>     } else {
>>>>>       return _configured_table_size;
>>>>>     }
>>>>>   }
>>>>>
>>>>>   ALWAYSINLINE Node** get_table() const {
>>>>>     if (SIZE != CONFIGURABLE_SIZE) {
>>>>>       return (Node**)(&_static_table[0]);
>>>>>     } else {
>>>>>       return _configured_table;
>>>>>     }
>>>>>   }
>>>>>
>>>>>   Node** lookup_node(unsigned hash, K const& key) {
>>>>>     unsigned index = hash % size();    <-----
>>>>>     Node** table = get_table();
>>>>>     Node** ptr = &table[index];   <-----
>>>>>
>>>>> back to the old code:
>>>>>
>>>>>   Node** lookup_node(unsigned hash, K const& key) {
>>>>>     unsigned index = hash % SIZE;      <-----
>>>>>     Node** ptr = &_table[index];       <-----
>>>>>
>>>>>
>>>>> If anyone has a better way of doing this, I'd love to hear it!
>>>>>
>>>>> Thanks!
>>>>> - Ioi
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>