RFR: 8114823: G1 doesn't honor request to disable class unloading

[Thomas Schatzl]

Hi Stefan,

On Fri, 2016-09-02 at 13:10 +0200, Stefan Johansson wrote:
> Hi,
> 
> Please review this fix for:
> https://bugs.openjdk.java.net/browse/JDK-8114823
> 
> Webrev:
> http://cr.openjdk.java.net/~sjohanss/8114823/hotspot.00/
> 
> Summary:
> For the G1 full GC there was no explicit code to handle when class 
> unloading was disabled. It worked for some cases because when 
> ClassUnloading is false classes in the system dictionary were kept
> alive  and never unloaded. But some types of classes, for example
> classes defined without a protection domain, were not kept alive.
> This fix changes the way we process roots in phase one of the full GC
> to include all class roots and also do the code cache scanning needed
> when classes aren't unloaded.
> 
> The fix adds similar argument parsing code as were previously added
> for CMS, so I moved them together instead of having to separate
> checks that do the same thing.
> 
> Testing:
> * RBT with -XX:-ClassUnloading on various test sets.
> * New regression test that previously failed now passes.
> * Verified that a few crashes I saw when running -ClassUnloading
> went 
> away with the fix.

  looks good except for one issue: The implementations for
process_all_roots() and process_all_roots_no_string_table() are
basically copy&paste with the only difference that the former has this
call to process the string table roots.

While reviewing this, this made it kind of hard to see the difference,
and I am kind of wary that this code duplication may cause issues at
some point.

I would strongly prefer if one method either called the other, or the
change added a _private_ third method with a flag to determine whether
string processing should be done in there.

If others object, I would be good with that too, but still see this as
unnecessary maintenance issue.

Thanks,
  Thomas