CRR (XS): 7098085: G1: partially-young GCs not initiated under certain circumstances

[Ramki Ramakrishna]

Hi Tony --

The changes look good.

On a side note, was the removal of the bit map clearing a different bug fix,
or is it somehow related to the issue described in the CR?
I can see that it is unnecessary and may be even wasteful,
but it seemed to me to be unrelated to the issue described in the CR,
unless I am missing something here.

As a related (but otherwise orthogonal) matter, it appears as though the 
idiom:-

    _sts.join();
    do_something();
    do_something_else();
   _sts_leave();

might gain from the usual constructor/destructor idiom used
elsewhere for code that wants to synchronize with other
parts of the system (in this case safepoints). For example:

   {
      SynchronizeWithSafepoints ss();     // or appropriate other name [1]
      do_something();
      do_something_else();
   }

[1] you will have to invent a suitable other name, perhaps 
DesynchronizeWithSafepoints
for when you do the reverse, i.e. perform an action outside of the _sts, 
as happens
when you are doing those synch barriers amongst the concurrent marking
threads in case of overflow and restart.

The benefit is that the extra scopes help clearly demarcate
the code that wants to be in the "critical section", if you
will, wrt safepoints that will want to synchronize wrt these
actions.

(As far as I can tell all invocations of join/leave or stsJoin/stsLeave
are paired and can become block-structured.)

The c'tor/d'tor idiom change is of course orthogonal to yr changes
and would likely want to be a separate CR, if you agree that this
may be another clean-up worth doing.

Otherwise looks good.
-- ramki

On 10/5/2011 2:21 PM, Tony Printezis wrote:
> Hi,
>
> I'd like to get a couple of people to have a look at the following 
> webrev:
>
> http://cr.openjdk.java.net/~tonyp/7098085/webrev.0/
>
> The code changes are very minor, most of the added lines are in fact 
> detailed comments on the changes.
>
> I'm going to do testing on two machines overnight, as this is quite 
> tricky code (even though it doesn't look it). But, I thought I'd open 
> the changes for code review early as the change is small and some 
> early feedback, if possible, would be greatly appreciated.
>
> Tony
>
> PS Correct url? Check. Text actually applies to the CR in the subject? 
> Check.
>