RFR (S): 7080389: G1: refactor marking code in evacuation pause copy closures

[Tony Printezis]

Of course I want to listen to what you have to say...

I understand your argument but I hope you can also see where I'm coming 
from. It's good to name the parameter with the action that it does and 
set it when it's appropriate (in this case for some closures during 
initial mark). Would you like better if John changes the comment to 
something like "we've been asked to mark the objects" and make sure the 
place where do_mark_forwardee is set to true describes why it is?

Tony

On 08/19/2011 10:43 AM, Stefan Karlsson wrote:
> On 08/19/2011 04:39 PM, Tony Printezis wrote:
>> You think "do_mark_forwardee" is generic?!?!?! It's very descriptive 
>> on what it does.
>
> Well, it's more generic than something like 
> do_mark_forwardee_during_initial_mark_root_scanning, which at least to 
> me, the comments imply. Anyways, you don't have to listen to my 
> arguments if you don't want to.
>
> StefanK
>
>>
>> Tony
>>
>> On 08/19/2011 10:35 AM, Stefan Karlsson wrote:
>>> Tony,
>>>
>>> On 08/19/2011 04:19 PM, Tony Printezis wrote:
>>>> Stefan,
>>>>
>>>> OK, good point. Maybe John can change the comment to something like 
>>>> "Need to mark the copied object if we're root scanning closure 
>>>> during initial mark, ....". Would this address your concern?
>>>
>>> I just don't see the reason for giving the parameter such a 
>>> "generic" name and then having comments about initial marking root 
>>> scan in the code whenever the parameter is used.
>>>
>>> StefanK
>>>
>>>>
>>>> Tony
>>>>
>>>> On 08/19/2011 10:01 AM, Stefan Karlsson wrote:
>>>>> But that's not what the comment says:
>>>>>
>>>>> 4339   // Need to mark the copied object if we're an initial
>>>>> 4340   // mark closure, or the object is already marked and
>>>>> 4341   // we need to preserve the mark.
>>>>> 4342   bool should_mark = do_mark_forwardee ||
>>>>> 4343       (_g1->mark_in_progress()&&  !_g1->is_obj_ill(obj));
>>>>>
>>>>> and
>>>>>
>>>>> 4357     // Object is not in collection set - if we're an initial 
>>>>> mark
>>>>> 4358     // closure then mark the object.
>>>>> 4359     if (do_mark_forwardee) {
>>>>>
>>>>> StefanK
>>>>>
>>>>>>
>>>>>> Tony
>>>>>
>>>
>