RFR: 8231153: Improve concurrent refinement statistics

[Thomas Schatzl]

Hi,

On 02.10.19 03:08, Kim Barrett wrote:
>> On Sep 25, 2019, at 5:40 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8231153
>>> Webrev:
>>> https://cr.openjdk.java.net/~kbarrett/8231153/open.00/
>>> Testing:
>>> mach5 tier1-5
>>> some local by-hand testing to look at the rate tracking.
> 
> I've made some updates to names, per some offline discussion with
> Thomas.  The main one is to use a "total_" prefix in various places
> for accumulated refinement time and accumulated number of cards
> refined.  Also backed out any "num_" prefix removals from the original
> change.  Removed the unsynchronized accumulation of the number of
> concurrently scanned cards.
> 
> I also fixed a bug in the card logging rate calculation.  The function
> I was using to get the end time for the last GC pause
> (last_known_gc_end_time_sec) doesn't do any such thing, but has a
> confusing name (JDK-8231638).
> 
> 
>> When reading the change I had the following thoughts to improve readability:
>>
[...]

Thanks a lot for considering my comments.

> 
>> - not sure, but I think exposing size() and start() and in G1FreeIdSet seems unnecessary: the only user is G1DirtyCardQueueSet anyway, and it is already owner of G1FreeIdSet. I.e. it knows these values already (and passes it to the initializer of the G1FreeIdSet instance, and already has a getter for the size() value), so getting it back from G1FreeIdSet seems a bit strange to me, but I am okay with current code.
> 
> I've backed out the changes to G1FreeIdSet, and instead introduced in
> G1DirtyCardQueue a private function providing the start index (always
> returning 0) and used it in the two relevant places, along with noting
> that there is code elsewhere that is assuming a 0 value.  That can be
> cleaned up later (JDK-8231734).
> 
> New webrevs:
> full: https://cr.openjdk.java.net/~kbarrett/8231153/open.01/
> incr: https://cr.openjdk.java.net/~kbarrett/8231153/open.01.inc/
> 
> Testing:
> mach5 tier1-5
> some local by-hand testing to look at the rate tracking.
> 

   looks good.

Thanks,
   Thomas