RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

[sangheon]

Hi Thomas,

On 04/11/2017 04:30 AM, Thomas Schatzl wrote:
> Hi all,
>
>    I would like you to ask for reviews of a set of related changes that
> optimize the way G1 updates/scans/refines cards of the remembered set.
>
> As these CRs are highly dependent on each other I would like to have
> them reviewed and pushed together. Another reason is that all of them
> are self-contained, which means that they take one or the other detour
> I want to avoid questions about.
>
> The basic premise is that refinement and Scan RS (and Update RS as it
> is mostly the same) phases are quite slow with G1. There are a lot of
> low-hanging fruits to be picked here, ranging from micro-optimizations,
> fixing "wrong" code sharing, re-using (slow) CMS code, doing stuff in
> non-straightforward ways etc.
>
> These changes try to make it better (tm) ;)
>
> Results are good, around 10%+ faster scan rs time/card, 10%+ faster
> update rs/card, refinement also faster (although it is too hard to
> measure), 8 LOC less (there is more to delete, but that one needs more
> care. Also these changes add some logging :))
>
> Here we go:
>
> 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for
> use during concurrent refinement and updating the rset
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8071280
> Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
>
> As the title indicates, this change allows specializations of
> HeapRegion::oops_on_card_seq_iterate_careful() according to current GC
> phase (concurrent or during gc) to allow to remove some checks.
> This is mostly a preparatory change for the next ones.
8071280, seems good to me.

src/share/vm/gc/g1/heapRegion.inline.hpp
  149   assert(ClassUnloadingWithConcurrentMark,

I have same question as Kim about this assertion.
In addition, block_size_during_gc() and block_size() are very similar 
except block_size() could return earlier.
Could you give me some explanation for this?

Thanks,
Sangheon


>
> 8162928: Micro-optimizations in scanning the remembered sets
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8162928
> Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
>
> This change removes the use of HeapRegionDCTOC during scanning
> remembered sets, and uses the faster
> HeapRegion::oops_on_card_seq_iterate_careful() (measured).
> HeapRegionDCTOC also depends CMS code which is just bad.
>
> 8177707: Specialize G1RemSet::refine_card for concurrent/during
> safepoint refinement
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177707
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>
> This change temporarily duplicates the code in G1RemSet::refine_card
> for during refinement/gc to allow (initial) specialization of these
> methods for their purpose.
>
> 8177044: Remove _scan_top from HeapRegion
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177044
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177044/
>
> Removes _scan_top and all its awkward handling by explicitly creating a
> snapshot of the top values of the heap regions. This allows significant
> specialization of the refine_during_gc() method by subsuming all the
> various checks at the beginning into a single one.
>
> This one is responsible for a large part of the improvement of the
> update rs phase.
>
> 8178148: Log more detailed information about scan rs phase
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8178148
> Webrev: http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
>
> Adds some more statistics gathering and logging about what the scan rs
> phase is actually doing, allowing better performance analysis. This
> information will also be used further in the future.
>
> 8175554: Improve G1UpdateRSOrPushRefClosure
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8175554
> Webrev: http://cr.openjdk.java.net/~tschatzl/8175554/webrev/
>
> (This mostly deletes code, so it's still "S" ;))
>
> Finally remove the G1ParPushHeapRSClosure and merge it with
> G1UpdateOrScanRSClosure. You may now notice that
> G1UpdateOrScanRSClosure is pretty similar to G1ParScanClosure - that is
> another issue.
> Also make sure that G1UpdateOrScanRSClosure properly does the humongous
> and "ext" region handling (which is benign so far).
>
> 8178151: Clean up G1RemSet files
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8178151
> Webrev: http://cr.openjdk.java.net/~tschatzl/8178151/webrev/
>
> Clean up after all these changes a little in the G1Remset* files,
> removing obsolete stuff, some renamings, etc.
>
> Thanks,
>    Thomas
>