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

[Thomas Schatzl]

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/

(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