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