RFR(M): 8248401: Refactor/unify RMI gc support functionality

[Stefan Karlsson]

On 2020-08-04 12:02, Thomas Schatzl wrote:
> Hi Stefan,
> 
>    thanks for your review.
> 
> On 03.08.20 13:30, Stefan Karlsson wrote:
>> Hi Thomas,
>>
>> Not a complete review, but regarding the ZGC change, I don't think 
>> it's appropriate to add a callback to Universe/CollectedHeap to 
>> ZStatCycle. Most functions in ZStat only collects and print 
>> statistics, and does not trigger external events. I think moving it to 
>> the call would be cleaner:
>>
>>    ~ZDriverGCScope() {
>>     ...
>>      // Update statistics
>>      ZStatCycle::at_end(_gc_cause, boost_factor);
>>
>>      // Update data used by soft reference policy
>>      Universe::update_heap_info_at_gc();
>>
>>      // Maybe add the call here?
>>      ...
>>    }
>>
> 
> Thank you for pointing out the appropriate place in ZGC. Done.
> 
>> Seeing this, it's also apparent that some of the "last_gc" info has 
>> historically been recorded in Universe:
>>
>> void Universe::update_heap_info_at_gc() {
>>    _heap_capacity_at_last_gc = heap()->capacity();
>>    _heap_used_at_last_gc     = heap()->used();
>> }
>>
>> while _last_whole_heap_examined_time_ns is recorded in CollectedHeap. 
>> I think we should create a RFR to move the heap variables out of 
>> Universe.
> 
> Done, JDK-8250961.
> 
> In addition to above changes, I tried to move the new call closer to 
> Universe::update_heap_info_at_gc() in a few places and improve naming of 
> some local variables.
> 
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8248401/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8248401/webrev.1 (full)

ZGC changes look good.

Thanks,
StefanK

> 
> Reran the test 300 times with these changes.
> 
> Thanks,
>    Thomas