RFR: 8242078: G1: Improve concurrent refinement analytics and logging

[Thomas Schatzl]

Hi,

On 09.04.20 03:57, Kim Barrett wrote:
>> On Apr 8, 2020, at 11:25 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>> Some minor issues:
>>
>> - G1CollectedHeap::gc_prologue - in new code doing timing measurements, please use Ticks/Tickspan instead of double/os::elapsedTime.
> 
> Done. I similarly updated the nearby calculation of prepare_tlab_time.
> We really need some helpers for Tickspan conversions, or should be
> recording Tickspans directly.
> 

Thanks. I have an old patch that does that (recording Tickspans/Ticks 
directly), however there is an issue with one measurement that is used 
both as duration and point in time which makes problems. Other than that 
it is a pretty large patch. I will try to find some time to look at it 
again in the next time.

>> - g1RemSetSummary.hpp:36: the "private" visibility specifier could be removed as well.
> 
> Done.
> 
>> - the modification to NonJavaThreadsList_lock maybe warrants a different change. It just does not seem to fit although it looks correct.
> 
> That change was for G1DetachedRefinementStats_lock, which I really
> don't want to have to put at a lower rank than leaf.  I probably
> should have called that out in the RFR email.
> 
> There is a related issue that I noticed but haven’t reported yet.  The description
> for BarrierSet::on_thread_{attach,detach} say that for Java threads they are
> called while holding the Threads_lock, but that isn’t true for detach.  It probably
> should be.
> 
>> - pre-existing: G1Analytics::predict_concurrent_refine-rate_ms does not seem to be used. Also predict_logged_cards_rate_ms. Could that (and all associated code) be removed so that the latter does not need to be renamed?
> 
> That stuff is needed for work in progress to improve the concurrent
> refinement thread control.
> 
>> Seems good otherwise.

Okay for all of the above.

> 
> Thanks.
> 
> New webrevs:
> full: https://cr.openjdk.java.net/~kbarrett/8242078/open.01/
> incr: https://cr.openjdk.java.net/~kbarrett/8242078/open.01.inc/
> 
> Testing: local (linux-x64) hotspot:tier1.
> 

Looks good.

Thomas