RFR: 8231153: Improve concurrent refinement statistics

Kim Barrett kim.barrett at oracle.com
Mon Oct 7 22:38:49 UTC 2019


> On Oct 7, 2019, at 8:57 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 2019-10-02 03:08, Kim Barrett wrote:
>> New webrevs:
>> full: https://cr.openjdk.java.net/~kbarrett/8231153/open.01/
>> incr: https://cr.openjdk.java.net/~kbarrett/8231153/open.01.inc/
> 
> The changes looks good, just one question around the calculation of total time and size.
> 
> src/hotspot/share/gc/g1/g1ConcurrentRefine.cpp
> ---
> 415 Tickspan G1ConcurrentRefine::total_refinement_time() const {
> ...
> 425   const_cast<G1ConcurrentRefine*>(this)->threads_do(&closure);
> 426   return closure._total_time;
> 427 }
> 428
> 429 size_t G1ConcurrentRefine::total_refined_cards() const {
> ...
> 439   const_cast<G1ConcurrentRefine*>(this)->threads_do(&closure);
> 440   return closure._total_cards;
> 441 }
> 
> Did you consider grouping these two functions into one, to avoid iterating the threads twice? Not sure this is a big deal, and it might only make the code more complicated, but it feels a bit unnecessary to do two iteration right after each other.

Thanks for the suggestion. I tried doing something like that in an
earlier version of this change, but I didn't like how it turned out.
But enough code has changed since then that I decided to try again.
This time seems okay.

So G1ConcurrentRefine now provides a RefinementStats class that
packages up the time and card counts, and a new function
total_refinment_stats() that returns one of those. Also removed
total_refinement_time() and total_refined_cards(), which are no longer
used. (If that were to change they are easily reinstated as wrappers
over total_refinement_stats().)

New webrevs:
full: https://cr.openjdk.java.net/~kbarrett/8231153/open.02/
incr: https://cr.openjdk.java.net/~kbarrett/8231153/open.02.inc/

Testing:
mach5 tier1





More information about the hotspot-gc-dev mailing list