[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