CRR (S): 7121623: G1: always be able to reliably calculate the length of a forwarded chunked array

[Tony Printezis]

Ramki,

On 12/23/2011 04:52 PM, Srinivas Ramakrishna wrote:
> >From sheer gut instinct (and nothing rational), it seems better to 
> try and manage the race in the reading of the size in the from-space 
> field (you know
> when you've read a negative size that you've lost a race; add 
> appropriate asserts to see that the object is forwarded,
> use the size from the post-copy image, and bail out), than it is to 
> mess with the length of the to-space image. Just sheer gut instinct -- 
> the pre-image is
> something we can play with once we have copied it (or have safely won 
> the race to copy it and know we are the
> only ones who will be modifying it). The post-copy image is the 
> pristine thing that we typically don't want to mess
> with lest we screw up. 

I understand what you're saying but I see this slightly differently. 
Currently, when we chunk arrays, we actually mutate both images:

from-image: length field maintains the next index
to-image: copy object, write mark word, and update ref fields as we 
process them and we find that they point to objects that have been / 
will be moved

It'd be nice if one of the images was actually pristine / consistent and 
currently neither is: each of them might be inconsistent at different 
times and for different reasons. Using the length field of the to-space 
object to encode the "next index" ensures that at least the from-image 
is always consistent and not mutated (apart from the forwarding 
reference of course). So, we can always reliably read the size from the 
from-image and we won't have to have any special code to detect which 
image is consistent and where to read the length from.

Regarding accidentally leaving the to-image length inconsistent: well, 
that'd be a bug and we'll have to fix it (heap verification will come 
across this sooner or later). If we don't update the length correctly 
that probably means that we also didn't process the last chunk on the 
array, which might leave some reference fields on it unprocessed, which 
is not a good situation anyway, length field being correct or not. :-)

Tony

> OK, I am waving my hands wildly and this is not always true. But 
> still, see if you can manage the
> race in the reading of the pre-image without undue cost.
>
> I'll think about it too at bed-time today :-)
>
> Happy Holidays!
> -- ramki
>
> On Fri, Dec 23, 2011 at 12:37 AM, Tony Printezis 
> <tony.printezis at oracle.com <mailto:tony.printezis at oracle.com>> wrote:
>
>     Hi all,
>
>     I have some bad news, it turns out that there's a subtle race in
>     the code. I've been testing the marking changes for quite a while
>     now (which rely on this patch) and I got a single failure. I'm
>     pretty sure it's the race I describe below. BTW, there's a
>     question at the end of the e-mail. :-)
>
>     Let's assume two threads, A and B, are racing to forward the same
>     object array X. Consider the following interleaving:
>
>     Thread A:
>     is X forwarded? no
>
>     Thread B:
>     is X forwarded? no
>     size = X.size();
>     X' = allocate size
>     try to forward X to X'? yes
>     copy X to X'
>     chunk X, X.length is negative
>
>     Thread A:
>     size = X.size(); -> BOOM, as size() just read a negative length
>
>     Interesting this race exists today but it's benign due to either
>     careful design or sheer luck. When A reads size = X.size() it will
>     actually get a smaller size than the actual size of X and will
>     allocate a chunk smaller than X (!!!). However, given that X is
>     already forwarded it won't need to copy it and will undo the
>     allocation. So, no harm done.
>
>     Question: Is there a reason why we use the from-space length field
>     to encode the next index instead of the to-space length field? If
>     we used the latter we can simplify the code by quite a lot. I
>     can't immediately think of any issues that this might cause.
>
>     Tony
>
>
>     On 12/21/2011 5:25 PM, Tony Printezis wrote:
>
>         Hi all,
>
>         I have two code reviews (thanks to John and Ramki!). Can I
>         push or is anybody else still looking at this?
>
>         Tony
>
>         On 12/14/2011 05:17 PM, Tony Printezis wrote:
>
>             Hi all,
>
>             Can I have a couple of code review for this small change?
>
>             http://cr.openjdk.java.net/~tonyp/7121623/webrev.0/
>             <http://cr.openjdk.java.net/%7Etonyp/7121623/webrev.0/>
>
>             The CR has a bit more explanation. The short version is
>             that I'm now encoding the "start index of the next chunk"
>             in the from-space length field of a chunked array (say
>             *that* quickly!) as a negative value to always be able to
>             distinguish it from the real length. This will simplify
>             the code for the CR John is currently working on (6484965)
>             but it's a small, self-contained change so it's good to
>             get it code reviewed and pushed separately.
>
>             Tony
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20111227/7a9c0dff/attachment.htm>