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.phillim...@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
