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

[Bengt Rutisson]

Ramki,

A comment inline...

On 2011-09-13 10:51, Ramki Ramakrishna wrote:
> 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)?

We tried to add some comments about this in the email that we just sent 
out as a reply to our earlier comments. Our theory is that any objects 
referred to from outside the collection set will be found through the 
remembered set. Thus, it should be enough to handle the heads of the 
lists and possibly anything in the collection set that the head directly 
refers to. The rest will be found through the remember set. Does that 
sound reasonable?


Stefan and Bengt

>
> 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/69d5ab74/attachment.htm>