[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: [12] RFR (S) 8205534: Remove SymbolTable dependency from serviceability agent
From:       Jini George <jini.george () oracle ! com>
Date:       2018-06-29 16:14:02
Message-ID: f9fe11de-33e3-8280-9347-ee4bc067f518 () oracle ! com
[Download RAW message or body]

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)                \

You could also remove the "friend class VMStructs" from the 
corresponding C++ data types.

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.

One nit:

Line 1079 of HeapHprofBinWriter.java: Extra spaces needed.

Thanks,
Jini.


On 6/23/2018 3:10 AM, coleen.phillimore@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
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic