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

[John Cuthbertson]

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
>