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

[Stefan Karlsson]

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

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.

StefanK

On 2020-07-23 12:54, Thomas Schatzl wrote:
> Hi all,
>
>   can I have reviews for this change that unifies RMI gc support 
> across collectors.
>
> Based on an idea that came up by Kim in recent reviews, instead of 
> every collector implementing that support by itself, the change moves 
> the details into CollectedHeap and the collectors only need to call an 
> appropriate method.
>
> That method has formerly called CollectedHeap::millis_since_last_gc() 
> which has a confusing name. I chose "last_whole_heap_examined_time" 
> instead which I believe is while long, more appropriate. I did not 
> find a good shorter name, I am open for issues.
>
> For testing I used implemented originally for JDK-8248401 (at 
> http://cr.openjdk.java.net/~tschatzl/8248401/webrev.test/). It still 
> is very much dependent on not having a too big delay between a GC and 
> the call to the RMI method. I do not know how to make it more stable 
> either, and fearing false positives I did not add it to this CR. I 
> could be convinced to do so if you think we should take the risk or 
> have a better idea on how it could be improved.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8248401
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8248401/webrev
> Testing:
> tier1-5; plus mentioned test
>
> Thanks,
>   Thomas