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

Kim Barrett kim.barrett at oracle.com
Thu Dec 6 01:03:37 UTC 2018


> 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.



More information about the hotspot-runtime-dev mailing list