RFR: 8183281: Remove unnecessary call to increment_gc_time_stamp

Erik Helin erik.helin at oracle.com
Fri Jun 30 09:37:12 UTC 2017


Hi all,

the following small patch removes an unnecessary call to 
increment_gc_time_stamp from 
G1CollectedHeap::do_collection_pause_at_safepoint (and the long, wrong, 
comment above the call).

We already do a call increment_gc_time_stamp much earlier in 
do_collection_pause_at_safepoint, which is enough. The reasons outlined 
in the comment motivating a second call is no longer true, the code has 
changed (but the comment has not).

Bug: https://bugs.openjdk.java.net/browse/JDK-8183281
Patch: see below
Testing: make hotspot

Thanks,
Erik

# HG changeset patch
# User ehelin
# Date 1498814642 -7200
#      Fri Jun 30 11:24:02 2017 +0200
# Node ID 62400b3cbec4e0d06e0d6c21c9486070d8c906a4
# Parent  10ccf0a5f63fdca04d9eda2c774ccdd0e12bc1a1
8183281: Remove unnecessary call to increment_gc_time_stamp

diff -r 10ccf0a5f63f -r 62400b3cbec4 src/share/vm/gc/g1/g1CollectedHeap.cpp
--- a/src/share/vm/gc/g1/g1CollectedHeap.cpp    Thu Jun 29 19:09:04 2017 
+0000
+++ b/src/share/vm/gc/g1/g1CollectedHeap.cpp    Fri Jun 30 11:24:02 2017 
+0200
@@ -3266,29 +3266,6 @@

          MemoryService::track_memory_usage();

-        // In prepare_for_verify() below we'll need to scan the deferred
-        // update buffers to bring the RSets up-to-date if
-        // G1HRRSFlushLogBuffersOnVerify has been set. While scanning
-        // the update buffers we'll probably need to scan cards on the
-        // regions we just allocated to (i.e., the GC alloc
-        // regions). However, during the last GC we called
-        // set_saved_mark() on all the GC alloc regions, so card
-        // scanning might skip the [saved_mark_word()...top()] area of
-        // those regions (i.e., the area we allocated objects into
-        // during the last GC). But it shouldn't. Given that
-        // saved_mark_word() is conditional on whether the GC time stamp
-        // on the region is current or not, by incrementing the GC time
-        // stamp here we invalidate all the GC time stamps on all the
-        // regions and saved_mark_word() will simply return top() for
-        // all the regions. This is a nicer way of ensuring this rather
-        // than iterating over the regions and fixing them. In fact, the
-        // GC time stamp increment here also ensures that
-        // saved_mark_word() will return top() between pauses, i.e.,
-        // during concurrent refinement. So we don't need the
-        // is_gc_active() check to decided which top to use when
-        // scanning cards (see CR 7039627).
-        increment_gc_time_stamp();
-
          if (VerifyRememberedSets) {
            log_info(gc, verify)("[Verifying RemSets after GC]");
            VerifyRegionRemSetClosure v_cl;



More information about the hotspot-gc-dev mailing list