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

[Tony Printezis]

Hi all,

Here's version 2 of this fix:

http://cr.openjdk.java.net/~tonyp/7121623/webrev.1/

I dropped the idea of using a negative value to encode the next index 
for the reasons I mentioned in previous e-mails. I now store the next 
index in the length field of the to-image, instead of the from-image, so 
that the from-image is always consistent and we can always calculate an 
object's size from it.

I also renamed some variables to make the code a bit more readable: old 
/ obj -> from_obj / to_obj.

As this fix is now conceptually different compared to the first version 
I'd like two new code reviews for it (having said that, the new set of 
changes are minimal compared to the previous version).

Thanks,

Tony

On 12/23/2011 03:37 AM, Tony Printezis 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/
>>>
>>> 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
>>>
>>>