RFR 8209387: Follow ups to JDK-8195100 Use a low latency hashtable for SymbolTable

[Gerard Ziemski]

> On Dec 5, 2018, at 7:03 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
>> On Dec 5, 2018, at 9:26 AM, Gerard Ziemski <gerard.ziemski at oracle.com> wrote:
>> 
>> 
>>> On Dec 4, 2018, at 6:20 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> 
>>>> On Dec 4, 2018, at 12:39 PM, Gerard Ziemski <gerard.ziemski at oracle.com> wrote:
>>>> 
>>>> Thank you Kim for the review!
>>>> 
>>>>> On Dec 4, 2018, at 12:36 AM, Kim Barrett <kim.barrett at oracle.com> wrote:What happened to this entry in the CR:
>>>>> 
>>>>> src/hotspot/share/classfile/stringTable.hpp
>>>>> 87 size_t add_items_count_to_clean(size_t ndead);
>>>>> 
>>>>> This name change doesn't seem right.
>>>> 
>>>> I changed:
>>>> 
>>>> add_items_count_to_clean() -> set_item_clean_count()
>>>> 
>>>> as part of the original work for JDK-8195100. Would you like to suggest a different name?
>>> 
>>> The name you describe is in SymbolTable.
>>> 
>>> Also as part of JDK-8195100, StringTable::add_items_to_clean() was changed to
>>> StringTable::add_items_count_to_clean().  That is the name change that I objected to at
>>> the time, and which is referred to by this item in the bug report.  Please revert it back to
>>> the original name.
>> 
>> I think that we should try to keep the names same in both String and Symbol tables. May I suggest that we either rename:
>> 
>> #1 StringTable: add_items_count_to_clean() -> set_item_clean_count()
>> 
>> #2 StringTable: add_items_count_to_clean() -> add_items_to_clean()     
>>  SymbolTable:     set_item_clean_count() -> add_items_to_clean()
>> 
>> I personally prefer #1 as we don’t actually add “items” themselves, but only their “count”.
> 
> They do different things, so it is unsurprising that they have different names.
> For StringTable, we’re accumulating a total from multiple threads.  “set_” prefix would be wrong here.
> For SymbolTable, we’re setting value from one thread’s work.
> 
> These are each part of a little “suite” of helper functions dealing with the number of items.
> I don’t love the existing names, but they are good enough.  But this one unmotivated name
> change stood out as inconsistent when I was doing the earlier review, so I requested that
> name change be backed out.  And that's still all I want.

In the end they do the exactly the same thing, i.e. update the dead counter that is used to calculate dead load factor, so that we know when to clean.

String table accumulates (which I think might be wrong, but need to think more about it), while Symbol table sets, so yeah that’s different, but the name “add_items_to_clean()” implies that we're adding “items” themselves, so personally I prefer “add_items_count_to_clean()”, but this discussion has gotten far enough. I reverted the String table API to what it used to be.

Issue:   https://bugs.openjdk.java.net/browse/JDK-8209387
Webrev:  http://cr.openjdk.java.net/~gziemski/8209387_rev3
Testing: Mach5 hs_tier1,2,3,4,5 in progress…


cheers