RFR (S): 8131319: Move G1Allocator::_summary_bytes_used back to G1CollectedHeap

[Thomas Schatzl]

Hi Mikael,

On Tue, 2015-07-21 at 14:57 +0200, Mikael Gerdin wrote:
> Hi Thomas,
> 
> On 2015-07-15 12:47, Thomas Schatzl wrote:
> > Hi all,
> >
> >    can I have comments and reviews for the following change to move
> > G1Allocator::_summary_bytes_used to a more fitting place?
> >
> > The problem from my POV is, that the member
> > G1Allocator::_summary_bytes_used, that contains the number of bytes held
> > by the heap outside of the allocators is located within G1Allocator.
> >
> > That sounds odd, particularly because G1Allocator actually never uses
> > it, it's managed completely by G1CollectedHeap.
> >
> > Before the addition of the G1ArchiveAllocator one could have made the
> > argument that G1Allocator holds all memory contained in the heap, but
> > this is not true any more: the memory occupied by G1ArchiveAllocator
> > needs to be added to the total sum of all managed space anyway, so
> > moving _summary_bytes_used out of G1Allocator seems sensible (to me).
> >
> > I talked to StefanJ about this change who originally moved
> > _summary_used_bytes into G1Allocator, and we could not find an argument
> > for keeping _summary_used_bytes in G1Allocator.
> >
> > This allows us to also refine the responsibilities of G1Allocator a
> > little more, to be the owner of the current allocation areas (regions)
> > only.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8131319
> >
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8131319/webrev/
> 
> The change looks good to me.
> Have you double-checked that the agent changes work (with jmap -heap)?

Thanks for the review.

Apparently not :(. I checked now, and there needs to be some further tiny
change in the SA code:

http://cr.openjdk.java.net/~tschatzl/8131319/webrev.0_to_1 (diff)
http://cr.openjdk.java.net/~tschatzl/8131319/webrev.1 (full)

Thanks,
  Thomas