RFR (S) 8223956: Make SymbolTable and StringTable AllStatic

gerard ziemski gerard.ziemski at oracle.com
Wed May 15 20:11:33 UTC 2019


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