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

[Srinivas Ramakrishna]

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).
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? 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?)

-- ramki

On Fri, Dec 23, 2011 at 1:52 PM, Srinivas Ramakrishna <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> 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/20111223/04b2b90d/attachment.htm>