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

Thomas Schatzl thomas.schatzl at oracle.com
Fri Feb 20 14:40:24 UTC 2015


Hi all,

  can I have reviews for the following cleanup? It removes the buffer
retain functionality of ParGCAllocBuffer.

It has (once I guess, but actually I am not sure because that must have
been long time ago) been used to retain the space at the end of the PLAB
for the next GC.

Nobody uses it. So that and all related code can be removed.

Here is a long description of changes:

ParGCAllocBuffer::flush_stats_and_retire() is always called with true
for "end_of_gc" and false for the "retain" parameter.
When moving that into that call to retire(), you will notice that
retire() is always called with retain=false.

Looking at ParGCAllocBuffer::retire(), if retain is always false,
_retained will never be true, which means that _retained_filler is never
used. With _retained always false, all of the asserts in retire() always
evaluate to true, so the end_of_gc parameter is useless.

(Actually the whole _retained_filler is never loaded from, making this
entire functionality non-working in the first place).

Now looking at what retire() does, there is actually a bug in
flush_stats_and_retire() because retire() adds to the waste (the area
between _top and _hard_top), but flush() is called first.

Looking at the code, we also only flush stats when ResizePLAB is set,
due to performance concerns. However this means that the output provided
by PrintPLAB will not be updated if ResizePLAB is not set, which is not
desirable at all (for comparison of fragmentation with ResizePLAB
enabled). I did not see the three atomic adds as a performance problem,
as this method is called once per PLAB per GC.

There is a small additional change in PLABStats::adjust_desired_plab_sz
where I merged the two separate "if (PrintPLAB)" statements.

Other cleanup changes include:
  - removed an obscure "XXX" comment
  - removed a PRAGMA
  - cleaned up includes (BlockOffsetTable - huh?)
  - tried to improve comments

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

Thanks,
  Thomas 




More information about the hotspot-gc-dev mailing list