RFR (S): 8131319: Move G1Allocator::_summary_bytes_used back to G1CollectedHeap
mikael.gerdin at oracle.com
Thu Jul 23 08:47:50 UTC 2015
On 2015-07-21 17:22, Thomas Schatzl wrote:
> 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)
>> 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)
Great! Thanks for verifying the SA code.
Looks good now.
More information about the hotspot-gc-dev