RFR (S): 8252034: G1: Remove *g1_reserved* methods

[Thomas Schatzl]

Hi,

On 20.08.20 01:55, Kim Barrett wrote:
>> On Aug 19, 2020, at 4:51 PM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi all,
>>
>>   can I have reviews for this change that removes the G1CollectedHeap::*g1_reserved* methods?
>>
>> They duplicate existing functionality actually just right below them.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8252034
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8252034/webrev/
>> Testing:
>> tier1
>>
>> Thanks,
>>   Thomas
> 
> I dislike that we still have two places (CollectedHeap::_reserved and
> HeapRegionManager) that have the reserved region information, and it seems
> hard to *prove* they are the same, even though they must be.
> 
> 

   so I already looked into this for the same reason:

- we initialize _reserved with the contents of heap_rs.
- we then (for whatever reason) make a copy of heap_rs to g1_rs
- we use g1_rs to initialize the heap

I will remove this indirection in JDK-8252086 out soon.

Now removing CollectedHeap::_reserved is another matter... so apart from 
some non-critical stuff (logging, assertions) it is basically only used 
by JFR to send out GCHeapSummary events, which basically report 
information about reserved spaces before/after gc.

(GCHeapSummary events are kind-of misleading because they assume 
contiguous spaces with contiguous allocations which does not apply to 
half of our collectors - I filed JDK-8252087 to look into).

I have a patch that replaces CollectedHeap::reserved() with a virtual 
method to implement, but CollectedHeap::_reserved *but* SA uses it so we 
won't get rid of it easily :(

 > But okay, looks good.

Thanks for your review.

Thomas