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