Request for review JDK-8151045, , Remove code duplication in PLABStats/G1EvacStats::adjust_desired_plab_sz

Kim Barrett kim.barrett at oracle.com
Fri Feb 17 22:19:50 UTC 2017


> On Feb 17, 2017, at 1:06 PM, Alexander Harlap <alexander.harlap at oracle.com> wrote:
> 
> Please review change for JDK-8151045 - Remove code duplication in PLABStats/G1EvacStats::adjust_desired_plab_sz
> 
> Change is located at http://cr.openjdk.java.net/~aharlap/8151045/webrev.00/
> 
> Common code is at (not virtual) PLABStats::adjust_desired_plab_sz() and class specific code is at new virtual helper method.
> 
> Alex

------------------------------------------------------------------------------
src/share/vm/gc/shared/plab.hpp
src/share/vm/gc/g1/g1EvacStats.hpp
 201   // helper for adjust_desired_plab_sz().
 202   virtual size_t adjust_desired_plab_sz_helper();

The new helper function should be protected, not public.

A better name for the new helper function would be
compute_desired_plab_sz.

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/plab.cpp
 168   if (_allocated == 0) {
 169     assert(_unused == 0,
 170            "Inconsistency in PLAB stats: "
 171            "_allocated: " SIZE_FORMAT ", "
 172            "_wasted: " SIZE_FORMAT ", "
 173            "_unused: " SIZE_FORMAT ", "
 174            "_undo_wasted: " SIZE_FORMAT,
 175            _allocated, _wasted, _unused, _undo_wasted);
 176 
 177     _allocated = 1;
 178   }

and
src/share/vm/gc/g1/g1EvacStats.cpp
  50   if (_allocated == 0) {
  51     assert((_unused == 0),
  52            "Inconsistency in PLAB stats: "
  53            "_allocated: " SIZE_FORMAT ", "
  54            "_wasted: " SIZE_FORMAT ", "
  55            "_region_end_waste: " SIZE_FORMAT ", "
  56            "_unused: " SIZE_FORMAT ", "
  57            "_used  : " SIZE_FORMAT,
  58            _allocated, _wasted, _region_end_waste, _unused, used());
  59     _allocated = 1;
  60   }

I think further improvement is possibleBy removing the near
duplication here.

(1) In both places remove the above quoted code.

(2) In the shared/plab version of the helper, protect the one
reference to _allocated against a zero value.

(3) In the main adjust function, add an assert
    assert(_allocated != 0 || _unused == 0, ...);

This loses the G1 specific _region_and_waste value if the assertion
fails, but keeping that doesn't seem worth the code duplication.

------------------------------------------------------------------------------




More information about the hotspot-gc-dev mailing list