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

[Stefan Johansson]

Thanks Mikael and Thomas for reviewing this,
StefanJ

On 2016-09-12 14:43, Mikael Gerdin wrote:
> 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
>>