RFR: 8232588: G1 concurrent System.gc can return early or late

[Kim Barrett]

> On Nov 12, 2019, at 8:19 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
> 
> Hi Kim,
> 
> Thanks for cleaning up this part of the code. Looks good overall but I have a couple of small comments below.

Thanks.

> On 2019-11-11 20:40, Kim Barrett wrote:
>>> On Nov 7, 2019, at 4:02 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> 
>>>> On Nov 7, 2019, at 4:58 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>>>> New webrevs:
>>>>> full: https://cr.openjdk.java.net/~kbarrett/8232588/open.01/
> 
> src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> ---
> 2089 // Return true if (x < y) with allowance for wraparound.
> 2090 static bool gc_counter_less_than(uint x, uint y) {
> 2091   return (x - y) > (UINT_MAX/2);
> 2092 }
> 
> This code makes me have to think to much, with regards to what it does. What do you think about using size_t instead of uint and just do simple comparisons?

The fanout from changing these counters from uint to size_t is pretty
large.  It also doesn't help for a long-running 32bit application.

I thought about adding a utility for this kind of thing, since this isn’t the only occurrence.
But I ran into problems for which I don’t currently have a solution (fixed by C++14).

> src/hotspot/share/runtime/mutexLocker.hpp
> ---
> 71 extern Monitor* G1FullGCCount_lock;              // in support of "concurrent" full gc
> 
> I think the name and the comment could be improve now when it's G1 specific. Something like G1OldGCCount_lock, to me Full signals that it's only STW.

The overloading of "full" (concurrent full vs stw full) occurs in many
places. But this particular area is using "old" rather than “full”, so
I've changed the name.

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