RFR (M): 8199742: Clean up state flags in G1CollectorState (original) (raw)
Thomas Schatzl thomas.schatzl at oracle.com
Mon Mar 26 15:06:52 UTC 2018
- Previous message (by thread): RFR (S): 8200233: Simple G1 evacuation path performance enhancements
- Next message (by thread): RFR (M): 8199742: Clean up state flags in G1CollectorState
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi all,
I would like to request reviews for this change that cleans up the flags in G1CollectorState, applying uniform naming, removing members that were basically temporary variables for a single method, and remove redundant flags (the four flags in the "XXX" block were basically a single one).
While this is just a step towards my goal of removing the flags as much as possible and merging them into enums, I think this is a very worthwhile step on that way. Further it blocks a few more changes of mine :P
Naming guidelines:
- flags indicating a particular GC were named "in_XXX_gc"
- durations spanning multiple garbage collections, i.e. phases have the "phase" postfix.
Other than that there were general updates for the names to hopefully make them reflect reality a bit closer.
Main changes apart from naming were
- in g1CollectionSet.cpp where I made add_young_region_common() to not update statistics. This avoids the need to make the full gc also a young gc (at least as far as flags are concerned); to fix this properly we would need to know the previous phase we were in when coming into the full gc. I just do not think it is worth adding a new state here (and the code has been wrong previously too, unconditionally adding these statistics to the young-only phase).
I made a note in the follow-up change JDK-8080226.
- the changes in G1Policy::record_collection_pause_end() were mostly due to removing the last_gc_was_young() member which was basically a reminder just for this method that the current gc was a young-only gc.
There is still _initiate_conc_mark_if_possible flag that indicates that a state change to initial mark should occur asap, but I think that should be moved to e.g. g1policy in a next change.
CR: https://bugs.openjdk.java.net/browse/JDK-8199742 Webrev: http://cr.openjdk.java.net/~tschatzl/8199742/webrev Testing: hs-tier 1-5
These changes are based on the rebuild remembered sets concurrently changes. As it may be a bit of work to get them working (I already pushed to first four), here is a webrev that updates latest jdk/hs to that level at http://cr.openjdk.java.net/~tschatzl/8199742/baseline/web rev/
Thanks, Thomas
- Previous message (by thread): RFR (S): 8200233: Simple G1 evacuation path performance enhancements
- Next message (by thread): RFR (M): 8199742: Clean up state flags in G1CollectorState
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]