Review request (S): 7021322: assert(object_end <= top()) failed: Object crosses promotion LAB boundary

[Stefan Karlsson]

On 09/13/2011 02:21 AM, Ramki Ramakrishna wrote:
> Sorry, that came out all wrong. The filling, as the comment says, is 
> necessary when
> the allocation is not from a PLAB but direct into the space. So that's 
> fine.
>
> The issue is when the object start is in the current PLAB, but its end 
> is not
> equal to the top. It seems as though it should not ever occur that the
> object start is in the PLAB but its end is anywhere other than the top of
> the PLAB. So it would be nice to check for that in the unallocate code.

I replaced:
assert(object_end <= top(), "Object crosses promotion LAB boundary");
...
if (object_end == top())

with:
assert(object_end == top(), "Not matching last allocation");

thanks for the review.
StefanK

>
> Sorry for the incorrect comments last time.
> -- ramki
>
> On 9/12/2011 5:14 PM, Ramki Ramakrishna wrote:
>> Good catch. Fix looks good.
>>
>> I am also a bit leery of any unallocate failing. It seems to me that 
>> each unallocate
>> should provably succeed. Thus the "return false" path seems bogus to 
>> me, and should
>> (in my book) be replaced with either an assert(false) or 
>> ShouldNotReachHere.
>> Then the object filling code becomes redundant and can be removed.
>>
>> On the other hand, may be I am missing some reason why the unallocate 
>> might
>> legitimately fail and the object filling becomes necessary in that case.
>>
>> Looks good otherwise.
>> -- ramki
>>
>> On 9/12/2011 1:49 PM, Stefan Karlsson wrote:
>>> http://cr.openjdk.java.net/~stefank/7021322/webrev/
>>>
>>> There's a bug in parallel scavenge when array chunking is used. 
>>> After a thread has succeeded in forwarding the pointer to a newly 
>>> copied array, it might change the length of the old object. This is 
>>> done as a part of the load balancing.
>>> If another thread races with the forwarding thread, it might read 
>>> the incorrect array length and copy just a part of the array. When 
>>> it later sees that the object has already been forwarded and calls 
>>> unallocate_object, it uses the original length of the array to 
>>> determine the size to unallocate.
>>>
>>> The fix is to pass the actual amount of memory that was allocated, 
>>> to unallocate_object.
>>>
>>> Tested with the failing test.
>>>
>>> StefanK