[12] RFR (S) 8205534: Remove SymbolTable dependency from serviceability agent

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Jul 3 17:55:48 UTC 2018


Thank you, Harold!
Coleen

On 7/3/18 1:36 PM, Harold David Seigel wrote:
> Hi Coleen,
>
> These changes look good.  Glad you got rid of a lot of code.
>
> Thanks, Harold
>
> On 7/3/2018 11:34 AM, coleen.phillimore at oracle.com wrote:
>>
>> Hi Jini,  Thank you for reviewing this.
>>
>> On 6/29/18 12:02 PM, Jini George wrote:
>>> Hi Coleen,
>>>
>>> Apologize for the delay. Your changes look good to me overall. A few 
>>> comments:
>>>
>>> It might make sense to also remove the corresponding lines in the 
>>> vmStructs files. Like:
>>>
>>>  File          Line
>>> vmStructs.cpp  170 typedef RehashableHashtable<Symbol*, mtSymbol> 
>>> RehashableSymbolHashtable;
>>> vmStructs.cpp  477 static_field(RehashableSymbolHashtable, _seed, 
>>> juint)                                 \
>>> vmStructs.cpp 1362 declare_type(RehashableSymbolHashtable, 
>>> BasicHashtable<mtSymbol>)     \
>>> vmStructs.cpp  475 static_field(SymbolTable, _the_table, 
>>> SymbolTable*)                          \
>>> vmStructs.cpp  476 static_field(SymbolTable, _shared_table, 
>>> SymbolCompactHashTable)                \
>>>
>>
>> Gerard has these changes in his changeset for rewriting the 
>> SymbolTable so I am going to leave this part of the change to him.
>>
>>> You could also remove the "friend class VMStructs" from the 
>>> corresponding C++ data types.
>>>
>>
>> Good point.  We'll make sure it's not there in his changes.
>>
>>> The test case: test/jdk/sun/tools/jhsdb/AlternateHashingTest.java 
>>> with the file: 
>>> test/jdk/sun/tools/jhsdb/LingeredAppWithAltHashing.java were created 
>>> to test the alternate hashing mechanism of the SymbolTable in SA. 
>>> Don't know if it makes sense to retain these.
>>>
>>
>> Ok, I was debating with myself whether to remove these.  It makes 
>> sense not to test something that doesn't test what's intended 
>> anymore.  I'll remove them.
>>
>>
>>> One nit:
>>>
>>> Line 1079 of HeapHprofBinWriter.java: Extra spaces needed.
>>>
>> Fixed.
>>
>> Thanks!
>> Coleen
>>> Thanks,
>>> Jini.
>>>
>>>
>>> On 6/23/2018 3:10 AM, coleen.phillimore at oracle.com wrote:
>>>> Summary: Modify SA code to not use SymbolTable and remove it.
>>>>
>>>> This is to support the concurrent hashtable for SymbolTable.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8205534.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8205534
>>>>
>>>> Tested with hs-tier1-5.
>>>>
>>>> Thanks,
>>>> Coleen
>>
>



More information about the hotspot-runtime-dev mailing list