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