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

[Tony Printezis]

On 12/23/2011 05:05 PM, Srinivas Ramakrishna wrote:
> Tony, So if I understand yr proposal, you would switch the roles of 
> the to and from copies' length fields (since you
> need both -- one to keep track of the original length and the other to 
> keep track of the scan finger in the array).

Correct.

> Of course there's a fwd'ing ptr from the the pre-image to the 
> post-image, but not vice-versa. So you'd still
> push the pre-image reference on the work queue, but use the length 
> field from the post-copy which you'd
> fetch and modify via the forwarding reference?

Correct.

> It is probably true that the post-image's length is not used
> during GC once it's been copied, but it'd be good to check (I'm 
> especially wary of CMS... but of course
> this is limited to G1 -- does G1 ever need to scan or iterate over 
> regions that are subject to being copied
> into during an incremental pause?)

This is of course something I was also worried about. In G1 we should 
not be scanning to-space objects that are being copied during GC, not 
only because the length might be incorrect due to this change but also 
because there are no guarantees that the objects are well formed 
(another thread might be in the process of copying them). For all 
regions we copy objects into we call save_marks() so that we never go 
over saved_mark() during scanning.

Tony

> -- ramki
>
> On Fri, Dec 23, 2011 at 1:52 PM, Srinivas Ramakrishna 
> <ysr1729 at gmail.com <mailto:ysr1729 at gmail.com>> 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. 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/d7cd7027/attachment.htm>