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

[Bengt Rutisson]

Hi Thomas,

On 2014-04-14 11:53, Thomas Schatzl wrote:
> 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.

Sounds good to file a CR for that. Good idea.

>
>> 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.

OK. Makes sense. Thanks for the extra information.

>
>> 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.

Right :)

Ship it!
Bengt

>
> Thanks,
>    Thomas
>
>