[12] RFR (S) 8205534: Remove SymbolTable dependency from serviceability agent
Harold David Seigel
harold.seigel at oracle.com
Tue Jul 3 17:36:05 UTC 2018
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