RFR (M) 8213753: SymbolTable is double walked during class unloading and clean up table timing in do_unloading

[coleen.phillimore at oracle.com]

On 1/30/19 5:17 AM, Aleksey Shipilev wrote:
> On 1/29/19 8:39 PM, coleen.phillimore at oracle.com wrote:
>> Summary: remove gc timing for short runtime cleanup triggering; make symbol table cleaning triggered
>> automatically on unloading
>>
>> Ran runThese with all Oracle GCs and got similar numbers of symbols unloaded.  Also ran tier1-5.
>>
>> See bug for more information.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/2019/8213753.01/webrev
> This looks fine. I tested Shenandoah just in case (it should not be affected, because it uses
> shared/parallelCleaning.* and SystemDictionary::do_unloading, like G1), it is still okay.

Thanks Aleksey for the testing.  I noticed Shenandoah didn't have the 
duplicate walk.
>
> Minor nits:
>
> *) Add spaces before format specifiers here?
>
>   698     log_debug(symboltable)("Concurrent work triggered, load factor:%f, items to clean:%s",
>   699                            get_load_factor(), has_items_to_clean() ? "true" : "false");

sure.
>
> *) In SystemDictionary::do_unloading, do we think that trigger_cleanup() are cheap? Otherwise it
> makes sense to retain a single GCTraceTime block around all three trigger_cleanups?

The three trigger cleanups aren't specifically ClassLoaderData, so I 
didn't include them in the timing.  I think people/tools might look for 
ClassLoaderData so I didn't want to change the name of the timing.   
Checking now...

     GCTraceTime(Debug, gc, phases) t("ClassLoaderData", gc_timer);

I thought of removing it completely but the timing but the enclosing 
timers (exception in shenandoah) also include timing 
CodeCache::do_unloading and clean_weak_klass_links which are more 
expensive than ClassLoaderData.

My original intention was to move the timers out of SystemDictionary 
completely though.  Maybe I should do that.

Thanks,
Coleen


> -Aleksey
>