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

[Mikael Gerdin]

Hi,

On 2016-09-12 14:16, Thomas Schatzl wrote:
> Hi Stefan,
>
> On Mon, 2016-09-12 at 10:49 +0200, Stefan Johansson wrote:
>> Thanks for looking at this Thomas,
>>
>> On 2016-09-09 13:43, Thomas Schatzl wrote:
>>>
>>> Hi Stefan,
>>>
>>> On Fri, 2016-09-02 at 13:10 +0200, Stefan Johansson wrote:
>>>>
>>>> Hi,
>>>> [...]
>>>    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.
>> I see your point, and I agree that from a maintenance point of view
>> it is better. Here's an example of how it would look:
>> Full: http://cr.openjdk.java.net/~sjohanss/8114823/hotspot.01/
>> Inc: http://cr.openjdk.java.net/~sjohanss/8114823/hotspot.00-01/
>>
>
> I like this better.
>
>> Let's see what other reviewers think.
>
> Okay.

I see Thomas' point and I think this is fine as well.
If we ever need to add one more boolean parameter to this code then I 
think we should throw it out and go with something like:

class G1RootClosures {
virtual OopClosure* string_table_oops() const = 0;
virtual OopClosure* cldg_strong_oops() const = 0;
virtual OopClosure* cldg_weak_oops() const = 0;
...
};

/Mikael

>
> Thanks,
>   Thomas
>