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

[Tony Printezis]

On 08/19/2011 11:58 AM, Stefan Karlsson wrote:
>> 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?
>
> Yes.
>
> I like the parameter name. The thing I'm having a problem with is that 
> the parameter name is "generic" in that sense that it doesn't convey 
> who's setting it to true. 

Whoever wants to make sure every object visited by the closure is marked.

Tony

> But then we have the comment that goes on and reveals that information 
> anyway.
>
> StefanK
>
>>
>> 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
>>>>>>>
>>>>>
>>>
>