RFR (S) 8223956: Make SymbolTable and StringTable AllStatic

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Thu May 16 11:04:50 UTC 2019


Thanks Robbin!
Coleen

On 5/16/19 3:58 AM, Robbin Ehn wrote:
> On 2019-05-15 22:11, gerard ziemski wrote:
>> Very nice.
>
> +1
>
> /Robbin
>
>>
>>
>> On 5/15/19 2:59 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> With a bit of encouragement from Robbin, I moved _local_table into 
>>> the .cpp files in symbolTable, stringTable and resolvedMethodTable. 
>>> This is quite nice because the hpp files no longer need to include 
>>> concurrentHashtable.hpp.
>>>
>>> Incr: 
>>> http://cr.openjdk.java.net/~coleenp/2019/8223956.02.incr/webrev/index.html
>>> Full: 
>>> http://cr.openjdk.java.net/~coleenp/2019/8223956.02/webrev/index.html
>>>
>>> Sanity testing in progress.
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 5/15/19 2:55 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>
>>>> On 5/15/19 2:48 PM, gerard ziemski wrote:
>>>>> hi Coleen,
>>>>>
>>>>> Looks great overall, really cleaned up the files nicely.
>>>>>
>>>>> Two small things:
>>>>>
>>>>> #1 you removed "private" from the stringTable.hpp at the top of 
>>>>> the class (line 51). I know the visibility is private by default, 
>>>>> by we still set it explicitly "private" regardless in other files 
>>>>> (ex. symbolTable.hpp), and we did it here before, so we should 
>>>>> follow the pattern (no need for new webrev for that)
>>>>
>>>> I don't think it's the coding style and I generally drop them 
>>>> whenever I can because it's redundant and noisy, so I'd rather not 
>>>> add it back.
>>>>
>>>>>
>>>>> #2 why did you drop "const" from SymbolTable::get_load_factor() ?
>>>>>
>>>> get_load_factor is now a static member fucnction so can't be const.
>>>>
>>>> Thank you for reviewing!  I'm glad you like the improvement.
>>>> Coleen
>>>>
>>>>>
>>>>>
>>>>> cheers
>>>>>
>>>>>
>>>>> On 5/15/19 9:40 AM, coleen.phillimore at oracle.com wrote:
>>>>>> Summary: Removed superfluous and confusing _the_table pointer.
>>>>>>
>>>>>> Ran hs tier1-3.
>>>>>>
>>>>>> open webrev at 
>>>>>> http://cr.openjdk.java.net/~coleenp/2019/8223956.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8223956
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>>
>>>>
>>>
>>>
>>



More information about the hotspot-runtime-dev mailing list