RFR (XS): 8140689: Skip last young-only gc if nothing to do in the mixed gc phase

Fri Oct 30 16:36:24 UTC 2015

Hi,

On Fri, 2015-10-30 at 11:40 -0400, Derek White wrote:
> FYI: I replied to the wrong RFR, so the following discussion applies to 
> 8140689.

Np.

> 
> Hi Thomas,
> 
> TL;DR; (r)eviewed! But I still have some questions!

Thanks for the review.

> 
> I'll top post here because I think I may misunderstand how things work.
> 
> I thought the current situation was something like this:
>   1) Concurrent Marking has started.

... in an initial mark young gc.

>   2) There are (potentially) a series of young-only GC happening 
> concurrently.
>   3) Concurrent Marking finishes. We call set_last_young_gc(true).

Which indicates that the next gc will be the last young-only gc before
the mixed phase.

>   4) A young-only GC finishes. Does concurrent marking happen during 
> young-only GC, or is it suspended?

I do not completely follow here: what is "a" young-only gc? As for the
concurrent marking, marking is always suspended during any STW pause.

>      - I have been assuming that 
> record_concurrent_mark_cleanup_completed() might be called during the 
> middle of a young-only GC.

No :)

>   5) As the young-only GC finishes in record_collection_pause_end(), we 
> realize that marking is finished and so if we need to we call 
> set_gcs_are_young(false) (meaning now in mixed-mode).

No. At the end of the last young-only gc, we switch over to mixed mode
now.

> But your comments below imply that a young-only gc has to start and 
> finish after step 3 before we can move to mixed mode collection.

Yes. The regular case sequence is always:

YO* -> YO_IM -> YO* -> YO_Last -> Mixed* -|
 ^                                        |
 |----------------------------------------|

Where * denotes 0..n repetitions. YO means young-only.

> I think the problem might be my assumption that a mixed-mode GC is 
> conceptually a GC with a slightly larger cset than a young-only GC. But 

No. Instead of filling up the available pause time with only young
regions (in any true young-only GC), G1 fills up available pause time
with some (less) young regions plus some old gen regions.

If we just added the old gen regions, we would most of the time break
the pause time goal.

> maybe the code is treating them as separate phases - a young-only GC 
> optionally followed by a partial old-gc. Even though the phrase "mixed" 
> implies that it encompasses at least two parts :-)
>
> Am I on the right track?

I think so. :)

> 
> Some more comments below...
> 
> On 10/30/15 6:48 AM, Thomas Schatzl wrote:
> > Hi Derek,
> >
> > On Thu, 2015-10-29 at 18:31 -0400, Derek White wrote:
> >> Hi Thomas,
> >>
> >> I've been thinking of "last_young_gc" as really "consider doing a mixed
> >> GC next time". So this gets set at the end of marking in
> > I would already consider the last young-only gc as part of the mixed
> > gc/reclamation phase. G1 only needs this gc to keep MMU. The problem is,
> > that if we count that gc directly as mixed, current abort conditions of
> > the mixed gc phase will be even more screwed up as they are now.
> >
> > We will look at these conditions in the future, but for now I would like
> > to avoid messing with that.
>
> OK.
>
> >> record_concurrent_mark_cleanup_completed(), and checked in
> >> record_collection_pause_end().
> >>
> >> So my question is why add a pre-filter in
> >> record_concurrent_mark_cleanup_completed() when I think the same
> >> condition is filtered out in record_collection_pause_end() any way?
> > Note that I am not sure you replied to the correct RFR mail here. I
> > think that question would fit more to the review for 8140689.
>
> YES. Sorry for the confusion.
>
> > Record_concurrent_mark_cleanup_completed is called before the last young
> > gc is started, so we can avoid it completely. We want that, because in
> > the RFR for 8140597 I made sure that the last young-only gc is not an
> > initial mark gc. Doing so reduces the number of states (now really only
> > "real" young-only gc can have an initial mark attached to it) which
> > makes the code simpler, and also reinforces the idea that the last
> > young-only gc is actually part of the mixed gc phase.
> >
> >> Another way to think of it is to scrap the notion of "last_young_gc"
> >> completely, because that may or may not be true based on the READER of
> >> that variable, not the SETTER. i.e. - You don't know if the current
> >> young_only GC is the last one until it is over, and then you immediately
> > That's not true. We already know before the start of the GC whether it
> > will be the last young-only gc or not.
> 
> Really? Because in record_collection_pause_end() we might decide not to 
> do any mixed-mode GC, so the next GC will also be young-only, so this 
> young-only gc wasn't really last after all.

"last young-only gc" means "last-before-mixed-gcs", where the amount of
mixed gcs may be zero :)

The decision to not do any mixed mode gc is mostly about ending the
mixed gc phase.

Maybe "last young-only gc" is just a big misnomer, describing the
collection set much more than the purpose.

> >> switch over to mixed_mode GC anyway, so I think it's more of a condition
> >> than a state.
> > I agree that we should try to scrap the last young-only gc by replacing
> > it with a proper mixed gc (that eventually does not reclaim any old gen
> > region). This however needs some consideration about the termination
> > conditions of the mixed gc phase.
> >
> > I would prefer if we could wait with these changes after properly
> > state'ifying the gc state. I.e. adding a state transition or removing
> > one does not involve looking very hard through a lot of (unrelated)
> > code.
> >
> >> Instead, rename "last_young_gc" to "concurrent_mark_completed". Then
> >> unconditionally and unambiguously call
> >> set_concurrent_mark_completed(true) in
> >> record_concurrent_mark_cleanup_completed(), and check
> > We do not want to unconditionally call
> > set_concurrent_mark_completed(true) in
> > record_concurrent_mark_cleanup_completed(), see RFR 8140689 :)
> >
> > In record_concurrent_mark_cleanup_completed(), if we already know that
> > the following mixed gc phase will reclaim nothing, do not start it. This
> > allows use to start another initial mark right after this one, making
> > sure that the marking cycle can start and complete asap in this
> > situation.
> >
> > Otherwise you will always have the intermediate last young-only gc which
> > may fill up the old gen to a point where G1 cannot recover without a
> > full gc. Which is the point of all these "get-to-initial-mark-asap"
> > changes.
>
> OK, I see the point to filtering out last_young_gc in 
> record_concurrent_mark_cleanup_completed().
>
> > I would at most rename it to something like "pre_mixed" or so :)
>
> I've been getting hung up on the name "last_young_gc". The phrase "last" 
> is not completely accurate, and isn't getting at what makes this young 
> collection special. If we can nail that difference down then the naming 
> should be simple.

It is an artifact of trying to keep MMU pause time and limitations in
the collection set selection for mixed gcs.

So consider an example: G1 just finished marking, in the previous
young-only gc G1 sized the target young/eden region number so that G1
can keep the pause time. At the time of marking, the young gen is
already too large to be able to process additional old gen regions.
Also, there is this rule, that for efficiencies' sake, G1 _always_ takes
at least a few old gen regions per mixed gc.
This means that the next GC, if we made it a regular GC, would
guaranteed to exceed the available pause time.

E.g. we know we can process 100 units of work in any kind of GC.
Processing an eden or survivor region takes 1 unit, processing an old
gen region takes 10 units.
Assume that at the time when the marking really completed, the mutator
already filled 85 regions/units. Now to make old gen reclamation
efficient, the policy is to take at least 5 old gen regions per single
mixed gc.
This means, that even if we did the GC right now, and disregarding
actual MMU, that GC would need to process 125 units, which obviously
fails the pause time goal.

For this reason, G1 just continues the current mutator phase, letting it
fill up the remaining available eden regions (i.e. 15), finishing that
"last young-only gc" in time, and then schedule the mixed gc using e.g.
a target young gen size of 50 regions (=50 units) and an old gen size of
5 regions (=50 regions).

Of course, that last young-only GC automatically checks whether there is
anything to reclaim in old gen too, and if so does not even request that
next mixed gc.
This particular change just moves this decision whether it's worth to
actually start that last young-only GC to the cleanup phase, because we
can, it also prepares the collection chooser (i.e. collection set
candidates).

I hope this makes the situation more clear, and particularly explains
why I think that last young-only gc is a non-optimal name due to its
similarity to the other young-only gcs, and that that gc actually
belongs to the mixed gc/reclamation phase.

There are two opportunities to remove that last young-only gc:
- if MMU allows, directly go into a mixed gc (but in some cases we still
need to do that last young-only gc).
- do not require mixed gc to take at least X old gen regions, at least
at the beginning. Find other, better conditions to end the mixed gc
phase.

Thanks,
  Thomas