RFR (M): 8067336: Allow that PLAB allocations at the end of regions are flexible

[Mikael Gerdin]

Thomas,

On 2015-08-24 15:43, Thomas Schatzl wrote:
> Hi Mikael,
>
> On Mon, 2015-08-24 at 13:18 +0200, Mikael Gerdin wrote:
>> Hi Thomas,
>>
>> On 2015-08-24 12:36, Thomas Schatzl wrote:
>>> Hi Tom,
>>>
>>>     thanks for the review.
>>>
>>> On Wed, 2015-08-19 at 18:19 -0400, Tom Benson wrote:
>>>> Hi Thomas -
>>>> Looks good to me.   Though there was one odd thing I noticed, not in the
>>>> changes but adjacent to one - this comment in g1Allocator.cpp:
>>>>
>>>>     273     if (buf != NULL) {
>>>>     274       // Otherwise.
>>>>     275       alloc_buf->set_buf(buf, actual_plab_size);
>>>>
>>>> There is another "Otherwise." comment just afterward at line 283, where
>>>> it makes sense, but the one at 274 looks left over from some past
>>>> restructuring.  Your call.  8^)
>>>> Tom
>>>
>>> I will remove the one in 274.
>>>
>>> New webrevs:
>>> http://cr.openjdk.java.net/~tschatzl/8067336/webrev.1_to_2/index.html
>>> (diff)
>>> http://cr.openjdk.java.net/~tschatzl/8067336/webrev.2/index.html (full)
>>
>> Overall I think this change is good.
>> I'm somewhat paranoid about output parameters, can you please add code
>> to initialize the value to be set by the out parameter to a dummy value
>> (-1?) and assert that it was properly set on the code paths where the
>> actual size is being used.
>>
>> The alignment of the \ in vmStructs_g1.hpp is wrong, I don't need to see
>> that in a new webrev though :)
>
>    thanks for the review. I fixed these issues in
>
> http://cr.openjdk.java.net/~tschatzl/8067336/webrev.3 (full)
> http://cr.openjdk.java.net/~tschatzl/8067336/webrev.2_to_3 (diff)

Looks good to me.
/Mikael

>
> The code sets the out parameter to zero and then checks whether it is
> within min/max after the call if the allocation had been successful.
>
> Testing:
> jprt
>
> Thanks,
>    Thomas
>
>