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

[Aleksey Shipilev]

On 1/30/19 2:27 PM, coleen.phillimore at oracle.com wrote:
>> *) 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 only (little) concern was that SD::do_unloading now has only one GCTraceTime("ClassLoaderData").
Without knowing beforehand if ::trigger_cleanup-s are cheap, it seems odd to drop GCTraceTime from
them. Something like:

  GCTraceTime(Debug, gc, phases) t("Trigger cleanups", gc_timer);

  ResolvedMethodTable::trigger_cleanup();

  if (unloading_occurred) {
     SymbolTable::trigger_cleanup();

     // Oops referenced by the protection domain cache table may get unreachable independently
     // of the class loader (eg. cached protection domain oops). So we need to
     // explicitly unlink them here.
     // All protection domain oops are linked to the caller class, so if nothing
     // unloads, this is not needed.
     _pd_cache_table->trigger_cleanup();
  }

...but that can be indeed skipped if we know that those triggers cost much less than the timer itself.

-Aleksey