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

[Ramki Ramakrishna]

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.

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