<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Hi John -- i looked at the webrev again and the new changes look
fine to me.<br>
I did have some comments regarding one issue which I had previously<br>
convinced myself was correct, but I have forgotten the reasoning, so
perhaps<br>
you can help jog my memory...<br>
<br>
On 9/12/2011 4:42 PM, John Cuthbertson wrote:<br>
<blockquote cite="mid:4E6E98DD.7000706@oracle.com" type="cite">On
09/12/11 06:22, Bengt Rutisson wrote:
<br>
...<br>
<blockquote type="cite">- Why is it necessary to go through the
references found during CM in
G1CollectedHeap::g1_process_strong_roots?
<br>
if
(!_process_strong_tasks->is_task_claimed(G1H_PS_refProcessor_oops_do))
{
<br>
// We need to treat the discovered reference lists of the
<br>
// concurrent mark ref processor as roots and keep entries
<br>
// (which are added by the marking threads) on them live
<br>
// until they can be processed at the end of marking.
<br>
ref_processor_cm()->weak_oops_do(&buf_scan_non_heap_roots);
<br>
}
<br>
<br>
</blockquote>
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.
<br>
</blockquote>
<br>
I agree with the reasoning you provide above, but i wonder now if
your<br>
code does do so today in its current form. Consider the following
scenario:-<br>
<br>
The CM reference processor has in its discovered list 3 reference
objects A->B->C->D.<br>
Let us say that A, B and D are not in the collection set and C
happens to be<br>
in the collection set of a partially young collection.<br>
<br>
Now the code above will check the head of this particular discovered
list and<br>
find that it's pointing to an object not in the discovered list and
will (if i understand<br>
the code correctly) immediately stop. C, which is a Reference object
that was copied and<br>
assumed to have been discovered, and thus did not have its referent
and next fields<br>
scanned can end up with dangling pointers if those objects were also
unlucky enough to<br>
be in the collection set.<br>
<br>
Am I missing something here that prevents this scenario (or allows
one to scan<br>
and preserve the objects that C refers to by some other means)?<br>
<br>
I am pretty sure we had discussed this before and I had convinced
myself that this worked at<br>
that time, but I am having a hard time remembering what the
reasoning was. (If nothing<br>
else, that reasoning should be written into the comment here for a
future time when we<br>
would have forgotten this email thread and your response to my
question :-)<br>
<br>
Now, if my fears are correct that the above code would sometimes
fail to preserve objects that<br>
we need to, we'd instead need to walk down the<b> entire</b> length
of each discovered list<br>
preserving their contents, and for very large old generations that
has the potential<br>
of becoming a serious scaling bottleneck (although clearly no worse
asymptotically<br>
than the remark pause which would need to do work that is
proportional to the same value)<br>
-- i just fear that one is a partially young pause of which there
may be potentially<br>
many for each remark pause in a concurrent cycle.<br>
<br>
What if, instead, one realized that this is necessary only for
partially young collections<br>
and one then treated any Reference object coming from a non-young
region<br>
normally. That is, just like other collectors' (young) reference
processors do,<br>
G1's STW young reference processor would never discover an object
that was not<br>
in the young gen. In that case, it would just copy all of (in my
example above)<br>
C's followers at the time it copied C, and we would not need to do
anything special<br>
to preserve C's followers.<br>
This also seems more naturally analogous to the "span" containment
checks<br>
that appear in the other collectors. So, while the exact mechanics
of how to<br>
embed this containment predicate into the reference processor
(replacing or<br>
generalizing the current role of span without compromising
performance for<br>
all the other collectors) can be worked out, it appears as though
conceptually<br>
this may lead to more uniform (wrt other existing collectors) and
simpler code.<br>
<br>
And a comment regarding ...<br>
<blockquote cite="mid:4E6E98DD.7000706@oracle.com" type="cite">
<blockquote type="cite">- Did you do any tests with
-XX:RefDiscoveryPolicy=1?
<br>
<br>
</blockquote>
No. Ramki has said that it has bit-rotted and he wishes to remove
it.
<br>
</blockquote>
<br>
I'll file a CR to remove it, if one does not already exist.<br>
I'll check on the hotspot-gc-use list that no one uses it (if they
do,<br>
they must be running buggy code; I do seriously believe this has
organically bit-rotted...)<br>
We might have to do the full CCC monty on this one, though, before
we can<br>
remove it (or we must fix it). I'll look into it and update.<br>
<br>
-- ramki<br>
<br>
<br>
</body>
</html>