RFR (not as big as it looks) 8184994: Add Dictionary size logging and jcmd

[coleen.phillimore at oracle.com]

On 8/1/17 10:53 AM, Aleksey Shipilev wrote:
> On 08/01/2017 04:36 PM, coleen.phillimore at oracle.com wrote:
>> Summary: added dcmd for printing system dictionary like the stringtable and symboltable and making
>> print functions go to outputstream rather than tty
>>
>> Tested with tier1 on linux x64, and runThese with jcmd to query systemdictionaries (lots of class
>> loaders).
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8184994.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8184994
> Cursory review:
>
> *) Not entirely clear why this include in compactHashtable.cpp:
>
>    32 #include "runtime/vmThread.hpp"

There's a VMThread::vm_thread() call in cmopactHashtable.cpp.  I took 
out #include diagnosticCommand.hpp from compactHashtable.hpp which 
transitively included it.

I should have mentioned that a lot of this change was to include files 
not transitively included by removing the dcmds from compactHashtable.hpp.
>
> *) I guess these pairs of lines may be coalesced in dictionary.cpp:
>
>   444   st->print_cr("^ indicates that initiating loader is different from "
>   445                "defining loader");
>
>   454       st->print("%4d: %s%s", index, is_defining_class ? " " : "^", e->external_name());
>   455       st->print(", loader ");
>
yes. that makes sense.

> *) dictionary.hpp: stray whitespace at the end
>
>   106   void print_on(outputStream* st) const ;

Fixed.

> *) placeholders.cpp: while we are here, probably worth changing print("\n") to cr()?

Yes, I should have looked at this more carefully to fix these and 
coalesce these lines below.
>
> *) placeholders.cpp: coalesce?
>
>   232       st->print("%4d: ", pindex);
>   233       st->print("placeholder ");

fixed.
>
> *) placeholders.hpp: method name is camel-cased, can rename it?
>
>   130   void printActionQ(outputStream* st) {

Sure.   I'll rename it print_action_queue().
>
> *) systemDictionary.cpp: in SystemDictionary::dump, the if(verbose) condition seems inverted. Should
> dump tables when verbose?

I kept the name dump_table from the VM.stringtable and VM.symboltable 
dcmd code, when the function really prints hashtable statistics.   I 
could rename that function to be print_table_statistics() instead.  And 
change ClassLoaderDataGraph::dump_dictionary to 
print_dictionary_statistics.  If that makes sense.

Thanks,
Coleen

>
>
> Thanks,
> -Aleksey
>