RFR (S): 8073466: Remove buffer retaining functionality and clean up in ParGCAllocBuffer

Kim Barrett kim.barrett at oracle.com
Mon Feb 23 05:40:53 UTC 2015


On Feb 20, 2015, at 9:40 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8073466
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8073466/webrev/
> testing:
> jprt, perf benchmarks with G1 and CMS

Looks good.

Thanks for the thorough description; that made the review pretty easy.

One minor comment, and a couple of pre-existing issues to perhaps do
something about.

------------------------------------------------------------------------------  
src/share/vm/gc_implementation/shared/parGCAllocBuffer.hpp
 148   void reset() {
 149     _allocated = 0;
 150     _wasted    = 0;
 151     _unused    = 0;
 152   }

This new private helper function seems to only be used in the one
place at the end of adjust_desired_plab_sz, where equivalent code used
to be inline.  I'm not sure making a helper function is actually
beneficial here.  The code that actually uses those variables is
earlier in the function.  Moving the reinitialization far away (and
losing the comment about clearing the accumulators) doesn't seem
helpful to me.

------------------------------------------------------------------------------   
src/share/vm/gc_implementation/shared/parGCAllocBuffer.hpp 
  57   // Minimum PLAB size.
  58   static const size_t min_size();
  59   // Maximum PLAB size.
  60   static const size_t max_size();

Pre-existing:

The const qualifier for the return types has no effect.

------------------------------------------------------------------------------  
src/share/vm/gc_implementation/shared/parGCAllocBuffer.cpp
  44   assert (min_size() > AlignmentReserve, "Inconsistency!");
  45   // ArrayOopDesc::header_size depends on command line initialization.
  46   AlignmentReserve = oopDesc::header_size() > MinObjAlignment ? align_object_size(arrayOopDesc::header_size(T_INT)) : 0;

Pre-existing:

The assert on line 44 seems oddly placed, given the one and only
assignment of AlignmentReserve immediately follows the assertion, and
that min_size() depends on AlignmentReserve.



More information about the hotspot-gc-dev mailing list