RFR (S) 8223956: Make SymbolTable and StringTable AllStatic

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Wed May 15 20:15:44 UTC 2019


Thanks Gerard!
Coleen

On 5/15/19 4:11 PM, gerard ziemski wrote:
> Very nice.
>
>
> 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