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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Jul 3 15:34:16 UTC 2018


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 serviceability-dev mailing list