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

[Stefan Karlsson]

On 2011-08-19 18:23, Tony Printezis wrote:
>
>
> 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.

I see that you're misunderstanding what I'm trying to say. I like the 
parameter name, but I don't like the comments.

StefanK

>
> 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
>>>>>>>>
>>>>>>
>>>>
>>