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

Thomas Schatzl thomas.schatzl at oracle.com
Mon Aug 24 13:43:48 UTC 2015


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)

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




More information about the hotspot-gc-dev mailing list