<!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">
<br>
Ramki,<br>
<br>
A comment inline...<br>
<br>
On 2011-09-13 10:51, Ramki Ramakrishna wrote:
<blockquote cite="mid:4E6F198C.5030707@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
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>
</blockquote>
<br>
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?<br>
<br>
<br>
Stefan and Bengt<br>
<br>
<blockquote cite="mid:4E6F198C.5030707@oracle.com" type="cite"> <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>
</blockquote>
<br>
</body>
</html>