RFR (S): 8213307: G1 should clean up RMT with ClassUnloadingWithConcurrentMark

[coleen.phillimore at oracle.com]

Thomas, thank you so much for cleaning this up.

http://cr.openjdk.java.net/~tschatzl/8213307/webrev.0_to_1/test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java.html

I think this test should have:
  * @requires vm.opt.final.ClassUnloading

The RMT cleanup (and now triggering) was put in 
SystemDictionary::do_unloading as a convenience rather than spread the 
call in all 4/5 GCs.  It's not really related to class unloading like 
the other cleanups.   If there's a better place for it, we can move it.

Also now that the timers are only timing triggering cleanup, it's not 
really interesting to have them for all the actions.  I think there 
should just be one timer around the whole SystemDictionary::do_unloading 
and not pass the flag.  I'll take your follow on RFE.

This change looks good apart from the comment above, which I don't need 
to see a webrev if you agree.

Thanks,
Coleen

On 11/5/18 11:09 AM, Aleksey Shipilev wrote:
> On 11/05/2018 05:02 PM, Erik Österlund wrote:
>>> Talking about surprises in current Hotspot GCs: ZGC does not even do class unloading, even if users
>>> "forces" it with -XX:+ClassUnloading(WithConcurrentMark) -- which is probably why we don't see ZGC
>>> config in that test.
>> We have not implemented STW class unloading in ZGC, because we did not think it is appropriate for a
>> concurrent collector to do that in a safepoint. We are going straight for concurrent class unloading
>> instead, which is currently in the process of integrating. The infrastructure being built to support
>> this is rather generic, and ultimately you will probably be able to hook both G1 and Shenandoah into
>> it if you like, on platforms supporting nmethod entry barriers.
> I understand that part. I was pointing out that "all current Hotspot GCs" are not necessarily
> reacting to class unloading flags in unsurprising manner: current ZGC being prime example today, and
> current Shenandoah would be another one, once it is integrated. Therefore, we better not set tests
> up assuming some particular "non-surprising" behavior.
>
> Thomas had reverted that part, anyway, so I don't think there is anything else to discuss.
>
> Cheers,
> -Aleksey
>