RFR (S): 8140597: Forcing an initial mark causes G1 to abort mixed collections

[Mikael Gerdin]

Hi Thomas,

On 2015-11-05 10:06, Thomas Schatzl wrote:
> Hi all,
>
>    here is the request for review for the updated and enhanced changes to
> avoid that forcing an initial mark (e.g. by humongous allocation, gc
> locker, metadata threshold reached, and other external requests) causes
> G1 to abort any current reclamation activity.
>
> Particularly in combination with adaptive IHOP (JDK-8136678) this will
> cause guaranteed full gcs if these requests are timed just right.
> Actually there is the possibility that G1 will never reclaim any space
> if there is e.g. a lot of gclocker activity (or lots of humongous
> objects).
>
> Anecdotally, I remember some case some time ago on hotspot-gc-use where
> we could not explain why G1 never did mixed gcs.
>
> This change modifies the way G1 treats such initial mark requests: it
> delays them until the mixed gc phase finishes - with JDK-8138740 G1 can
> then even start the initial mark right after the mixed gc (as opposed to
> need an intermediate young-only GC inbetween).
>
> I think this change in behavior is justified due to the assumption that
> when we already did the liveness analysis/marking we should definitely
> use this information.
>
> To hopefully speed up reviews some run-down of the changes:
>
> G1CollectorPolicy::about_to_start_mixed_phase(): new method that defines
> the marking cycle to span both the marking and the last young-only gc.
> This prevents initial mark requests to skip the following mixed gc phase
> if they occur between when marking actually finished, and the last
> young-only gc.
>
> This method is used in need_to_start_conc_mark() that schedules a
> regular initial mark (since we already running marking, no need to do it
> again), and decide_on_conc_mark_initiation(), which actually switches to
> the initial mark.
>
> G1CollectorPolicy::decide_on_conc_mark_initiation()'s purpose is to
> switch to initial mark. Previously, it allowed that even during mixed gc
> (and during the last young-only gc), simply aborting the mixed gcs.
> Instead, G1 now disallows switching to initial-mark from initial-mark to
> the last mixed gc. This obviates the need for the code that aborts mixed
> gcs.
>
> All other changes are comments, asserts and a minor cleanup in
> whitebox.cpp.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8140597
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8140597/webrev
> Testing:
> jprt, manual verification of correct state transitions, vm.gc testlist

I think this change makes sense in general and from my understanding we 
will not skip an initial mark but rather we will delay it until it's 
appropriate to start a new cycle.

This is because we only clear initiate_conc_mark_if_possible
if (!about_to_start_mixed_phase() && collector_state()->gcs_are_young())

Given that my understanding is correct, Looks good :)

/Mikael

>
> Thanks,
>    Thomas
>
>