RFR(M/L): 6484982: G1: process references during evacuation pauses

[Ramki Ramakrishna]

Hi John -- i looked at the webrev again and the new changes look fine to me.
I did have some comments regarding one issue which I had previously
convinced myself was correct, but I have forgotten the reasoning, so perhaps
you can help jog my memory...

On 9/12/2011 4:42 PM, John Cuthbertson wrote:
> On 09/12/11 06:22, Bengt Rutisson wrote:
> ...
>> - Why is it necessary to go through the references found during CM in 
>> G1CollectedHeap::g1_process_strong_roots?
>>     if 
>> (!_process_strong_tasks->is_task_claimed(G1H_PS_refProcessor_oops_do)) {
>>       // We need to treat the discovered reference lists of the
>>       // concurrent mark ref processor as roots and keep entries
>>       // (which are added by the marking threads) on them live
>>       // until they can be processed at the end of marking.
>>       ref_processor_cm()->weak_oops_do(&buf_scan_non_heap_roots);
>>     }
>>
> Reference objects, currently, can only be discovered by one reference 
> processor and there is currently no mechanism for moving a reference 
> from one RP to another. References discovered by the CM reference 
> processor are processed during remark. So if we have a reference 
> object that has already been discovered by the CM reference processor, 
> which is also in the collection set, then it will not be 'discovered' 
> by the STW reference processor. But the routine 
> ReferenceProcessor::discover_reference() will return false (meaning 
> that the calling code does not need to treat the reference object as a 
> regular oop) and the strongly reachable graph from the reference 
> object will not be walked during the evacuation clause.  So we have to 
> ensure that the reference and it's referent are preserved until 
> marking completes; the CM ref processor will then process the 
> reference. If we had some way of encoding the reference processor 
> instance into the discovered reference object and then also check 
> against that in ReferenceProcessor::discover_reference() then we could 
> do without this code.

I agree with the reasoning you provide above, but i wonder now if your
code does do so today in its current form. Consider the following scenario:-

The CM reference processor has in its discovered list 3 reference 
objects A->B->C->D.
Let us say that A, B and D are not in the collection set and C happens to be
in the collection set of a partially young collection.

Now the code above will check the head of this particular discovered 
list and
find that it's pointing to an object not in the discovered list and will 
(if i understand
the code correctly) immediately stop. C, which is a Reference object 
that was copied and
assumed to have been discovered, and thus did not have its referent and 
next fields
scanned can end up with dangling pointers if those objects were also 
unlucky enough to
be in the collection set.

Am I missing something here that prevents this scenario (or allows one 
to scan
and preserve the objects that C refers to by some other means)?

I am pretty sure we had discussed this before and I had convinced myself 
that this worked at
that time, but I am having a hard time remembering what the reasoning 
was. (If nothing
else, that reasoning should be written into the comment here for a 
future time when we
would have forgotten this email thread and your response to my question :-)

Now, if my fears are correct that the above code would sometimes fail to 
preserve objects that
we need to, we'd instead need to walk down the*entire* length of each 
discovered list
preserving their contents, and for very large old generations that has 
the potential
of becoming a serious scaling bottleneck (although clearly no worse 
asymptotically
than the remark pause which would need to do work that is proportional 
to the same value)
-- i just fear that one is a partially young pause of which there may be 
potentially
many for each remark pause in a concurrent cycle.

What if, instead, one realized that this is necessary only for partially 
young collections
and one then treated any Reference object coming from a non-young region
normally. That is, just like other collectors' (young) reference 
processors do,
G1's STW young reference processor would never discover an object that 
was not
in the young gen. In that case, it would just copy all of (in my example 
above)
C's followers at the time it copied C, and we would not need to do 
anything special
to preserve C's followers.
This also seems more naturally analogous to the "span" containment checks
that appear in the other collectors. So, while the exact mechanics of how to
embed this containment predicate into the reference processor (replacing or
generalizing the current role of span without compromising performance for
all the other collectors) can be worked out, it appears as though 
conceptually
this may lead to more uniform (wrt other existing collectors) and 
simpler code.

And a comment regarding ...
>> - Did you do any tests with -XX:RefDiscoveryPolicy=1?
>>
> No. Ramki has said that it has bit-rotted and he wishes to remove it.

I'll file a CR to remove it, if one does not already exist.
I'll check on the hotspot-gc-use list that no one uses it (if they do,
they must be running buggy code;  I do seriously believe this has 
organically bit-rotted...)
We might have to do the full CCC monty on this one, though, before we can
remove it (or we must fix it). I'll look into it and update.

-- ramki


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20110913/526559b2/attachment.htm>