RFR(S): 7143490: G1: Remove HeapRegion::_top_at_conc_mark_count

[Bengt Rutisson]

Hi John,

Thanks for the very detailed explanation! Sounds really good.

Ship it!
Bengt

On 2012-04-06 23:19, John Cuthbertson wrote:
> Hi Bengt,
>
> Thanks for looking over the changes. Your question is a good one. 
> Hopefully I can answer it to your satisfaction.
>
> The original liveness counting code walked marking bitmap (i.e. the 
> set bits) for the live objects between _top_at_conc_mark_count and 
> NTAMS. At the start of marking, _top_at_conc_mark_count was set to 
> start of the region.  In the event of an evacuation failure while 
> evacuating the objects in a region, R, not all the objects will have 
> been evacuated. So we would tell the counting code to revisit this 
> region bey setting _top_at_conc_mark_count to the region bottom. Any 
> non-evacuated explicitly marked objects would then be walked by the 
> counting code. Any explicitly marked objects that were successfully 
> evacuated would have had their marks propagated so that the new 
> location was marked, and the old marks would have been cleared.
>
> As a result of Tony's marking changes this whole mechanism changed. 
> Now if an evacuation failure occurs during an initial mark pause - any 
> non-evacuated objects in the region are explicitly marked and NTAMS is 
> set to top. Successfully evacuated objects will be copied above NTAMS 
> of the to-space region. When the non-evacuated objects are marked - 
> they will be included in the liveness counting. Objects that are 
> copied above NTAMS are included during the finalization of the 
> liveness counting during cleanup.
>
> The finalization of the liveness counting did include the range 
> [_top_at_conc_mark_count, NTAMS), but since the remark pause set 
> _top_at_conc_mark_count to NTAMS this interval was usually empty. If 
> we did get an evacuation failure during marking (and post remark) then 
> _top_at_conc_mark_count would have been reset. The only issue with 
> this is that we would include the objects in the region that got the 
> evacuation failure twice in the liveness counting data (once as a 
> result of an explicit mark (or successful copy) and the other from the 
> finalization code). This would look like we had more live data than we 
> actually did. When the original liveness counting changes were added 
> we knew that we would lose some accuracy. Removing the field will 
> reduce this inaccuracy.
>
> Does this answer your question?
>
> Thanks,
>
> JohnC
>
> On 04/03/12 06:04, Bengt Rutisson wrote:
>>
>> Hi John,
>>
>> I think your changes look good.
>>
>> Just a question for my understanding. You say "The value of 
>> _top_at_conc_mark_count is now the same as NTAMS". You are probably 
>> correct, but in HeapRegion::note_self_forwarding_removal_start() the 
>> old code was:
>>
>>  if (during_initial_mark) {
>>     ...
>>     _next_top_at_mark_start = top();
>>     set_top_at_conc_mark_count(bottom());
>>     ...
>>
>> Here we set NTAMS and _top_at_conc_mark_count() to different values. 
>> I assume that this did not have any effect, but can you explain why?
>>
>> Thanks,
>> Bengt
>>
>> On 2012-04-03 01:45, John Cuthbertson wrote:
>>> Hi Everyone,
>>>
>>> Here's my contribution to the cleanup week.
>>>
>>> Can I have a couple of volunteers review the changes for this CR? 
>>> The webrev can be found at: 
>>> http://cr.openjdk.java.net/~johnc/7143490/webrev.0/
>>>
>>> Summary:
>>> As a result of Tony's recent marking changes (associated with the 
>>> marking of survivors) the field HeapRegion::_top_at_conc_mark_count 
>>> is no longer required. The value of _top_at_conc_mark_count is now 
>>> the same as NTAMS. Additionally I simplified the verification code 
>>> that sets the bits in the live card bitmap and refactored the 
>>> closures that finalize and verify the liveness counting data to 
>>> avoid code duplication.
>>>
>>> Testing: I inserted a 500ms sleep in the marking cycle between the 
>>> remark and cleanup pauses to ensure that at least one evacuation 
>>> pause was seen during this phase, and ran the GC test suite with 
>>> several marking thresholds (2, 5, and 10%) and verification enabled.
>>>
>>> Thanks,
>>>
>>> JohnC
>>
>