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

Thomas Schatzl thomas.schatzl at oracle.com
Mon Nov 12 14:31:41 UTC 2018


Hi Coleen,

  thanks for your review.

On Mon, 2018-11-12 at 09:15 -0500, coleen.phillimore at oracle.com wrote:
> 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 test executes its own VM via processtools, and we make sure that
that one has class unloading enabled. So I do not think the test itself
requires an @requires.

> 
> 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.

Yes, I will follow up with an RFE (I need to check if I already filed
one).

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

Unfortunately, not ;) Can you look over the test again and agree with
me that the VM where we actually test the RMT cleanup is guaranteed to
run with class unloading enabled so that the code is executed?

Thanks,
  Thomas




More information about the hotspot-runtime-dev mailing list