RFR (XS): 8036860: Pad and cache-align the BiasedMappedArray

[Bengt Rutisson]

On 2014-03-14 12:23, Stefan Karlsson wrote:
>
> On 2014-03-14 12:10, Thomas Schatzl wrote:
>> Hi Stefan,
>>
>>   thanks for the review!
>>
>>
>> On Fri, 2014-03-14 at 10:51 +0100, Stefan Karlsson wrote:
>>> On 2014-03-14 10:35, Thomas Schatzl wrote:
>>>> Hi,
>>>>
>>>>     I messed up the webrev link (and the CR number in the topic) 
>>>> due to
>>>> two very similarly named CRs I currently have assigned to me.
>>>>
>>>> Here is the correct webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~tschatzl/8036860/webrev/
>>> http://cr.openjdk.java.net/~tschatzl/8036860/webrev/src/share/vm/gc_implementation/g1/g1BiasedArray.hpp.udiff.html 
>>>
>>>
>>> Could you move create_new_base_array(size_t length, size_t 
>>> elem_size) to
>>> the .cpp file, so that we don't have to include padded.inline.hpp in 
>>> the
>>> .hpp file?
>>>
>>> The allocation.inline.hpp include should probably be chanted to
>>> allocation.hpp.
>> Actually, with the move of create_new_base_array() these include
>> statements could be removed completely. And no allocation include file
>> is needed any more.
>>
>>>
>>> http://cr.openjdk.java.net/~tschatzl/8036860/webrev/src/share/vm/memory/padded.inline.hpp.udiff.html 
>>>
>>>
>>> +  return (T*)align_size_up_((uintptr_t)chunk, alignment);
>>>
>>> Could you use align_pointer_up instead, as you did in 8035815?
>> Done.
>>
>>>
>>> Other than that, this looks good.
>> All fixed.
>>
>> New complete webrev
>> http://cr.openjdk.java.net/~tschatzl/8036860/webrev.1/
>>
>> Diff to last:
>> http://cr.openjdk.java.net/~tschatzl/8036860/webrev.1.diff/
>
> Looks good.

Looks good to me too.

Bengt

>
> StefanK
>
>>
>> (Note that in this diff webrev,
>> G1BiasedMappedArrayBase::create_new_base_array() shows the comment
>> wrongly indented. This has been fixed in the "complete" webrev above
>> already. I only noticed that after looking at the diffs for a final
>> time, but I had already merged back the changes from your review to the
>> patch).
>>
>> Thanks,
>> Thomas
>>
>>
>