RFR (S): JDK-8028710: G1 does not retire allocation buffers after reference processing work

[Thomas Schatzl]

Hi,

On Mon, 2014-04-14 at 10:43 +0200, Bengt Rutisson wrote:
> Hi Thomas,
> 
> Nice change. Looks good.

Thanks.

> Moving retire_alloc_buffers() to the destructor of G1ParScanThreadState 
> is nice, but has the side effect that in 
> G1CollectedHeap::process_discovered_references() the time to retire the 
> alloc buffers is not included anymore in the reference processing time 
> reported by that method. I guess this time is bounded and hopefully 
> short, but do you think it could be an issue?

Retiring an allocation buffer does two things: adding a few values to
statistics (wasted space), and inserting the filler block into the heap.

I do not expect either to be very expensive.

Also, I would like to remove use of separate allocation buffers for the
different gc phases at some point: it is very expensive in terms of
memory usage to size them the same in phases that have a different
allocation behavior.

Particularly in the soft-reference processing phase, very often only a
few objects are kept alive, although G1 sizes the buffer to the same
size as for the actual evacuation phase which has comparatively much
more survivors.

I will probably create a CR for that.

> I assume you had to change the initial value of _retired in 
> G1ParGCAllocBuffer from false to true because of the assert in 
> ~G1ParGCAllocBuffer(). Can you elaborate a bit on why we create a 
> G1ParGCAllocBuffer without using and retiring it? Basically, why is the 
> initial value for _retired not false?

We may create G1ParGCAllocBuffers for G1ParScanThreadState instances
that never allocate anything, e.g. if there are no survivors in the
first GC phase, or if reference processing does not keep alive any
objects (or there were no soft references in the first place).

As for initializing _retired to true, this follows from retire() setting
_retired to this value. I.e. the initial state of G1ParGCAllocBuffers
should be exactly the same state as after retirement.

> Nice that there were no performance impact of making the methods in 
> ParGCAllocBuffer virtual.

That also is true for making allocate() virtual btw - this might be
handy in the future.

Thanks,
  Thomas