From 99698adb9c100dc67d0cb0e239c410e9d0bc8e12 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Sat, 7 Dec 2024 09:14:47 -0800
Subject: [PATCH 01/18] Reset bitmap after collection

---
 .../gc/shenandoah/shenandoahConcurrentGC.cpp  | 40 +++++++++++
 .../gc/shenandoah/shenandoahConcurrentGC.hpp  |  5 ++
 .../share/gc/shenandoah/shenandoahFullGC.cpp  |  2 +-
 .../gc/shenandoah/shenandoahGeneration.cpp    | 68 ++++++++++++-------
 .../gc/shenandoah/shenandoahGeneration.hpp    |  2 +-
 .../gc/shenandoah/shenandoahHeapRegion.cpp    |  1 +
 .../gc/shenandoah/shenandoahHeapRegion.hpp    | 14 ++++
 .../share/gc/shenandoah/shenandoahOldGC.cpp   |  3 +
 .../gc/shenandoah/shenandoahPhaseTimings.hpp  |  1 +
 9 files changed, 108 insertions(+), 28 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index b1c2e72ef82b6..3b90116b3afa0 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -236,6 +236,11 @@ bool ShenandoahConcurrentGC::collect(GCCause::Cause cause) {
   if (heap->mode()->is_generational()) {
     ShenandoahGenerationalHeap::heap()->complete_concurrent_cycle();
   }
+
+  // Instead of always reset before collect, some reset can be done after collect to save
+  // the time before before the cycle so the cycle can be started as soon as possible.
+  entry_reset_after_collect();
+
   return true;
 }
 
@@ -578,6 +583,16 @@ void ShenandoahConcurrentGC::entry_cleanup_complete() {
   op_cleanup_complete();
 }
 
+void ShenandoahConcurrentGC::entry_reset_after_collect() {
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  TraceCollectorStats tcs(heap->monitoring_support()->concurrent_collection_counters());
+  const char* msg = conc_reset_after_collect_event_message();
+  ShenandoahConcurrentPhase gc_phase(msg, ShenandoahPhaseTimings::conc_reset_after_collect);
+  EventMark em("%s", msg);
+
+  op_reset_after_collect();
+}
+
 void ShenandoahConcurrentGC::op_reset() {
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
   if (ShenandoahPacing) {
@@ -1187,6 +1202,23 @@ void ShenandoahConcurrentGC::op_cleanup_complete() {
   ShenandoahHeap::heap()->recycle_trash();
 }
 
+void ShenandoahConcurrentGC::op_reset_after_collect() {
+  ShenandoahWorkerScope scope(ShenandoahHeap::heap()->workers(),
+                          ShenandoahWorkerPolicy::calc_workers_for_conc_reset(),
+                          "reset after collection.");
+
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+  if (heap->mode()->is_generational()) {
+    if (!_do_old_gc_bootstrap) {
+      // Only reset for young generation, bitmap for old generation must be retained,
+      // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
+      heap->young_generation()->reset_mark_bitmap(false, false);
+    }
+  } else {
+    _generation->reset_mark_bitmap(false, false);
+  }
+}
+
 bool ShenandoahConcurrentGC::check_cancellation_and_abort(ShenandoahDegenPoint point) {
   if (ShenandoahHeap::heap()->cancelled_gc()) {
     _degen_point = point;
@@ -1236,6 +1268,14 @@ const char* ShenandoahConcurrentGC::conc_reset_event_message() const {
   }
 }
 
+const char* ShenandoahConcurrentGC::conc_reset_after_collect_event_message() const {
+  if (ShenandoahHeap::heap()->unload_classes()) {
+    SHENANDOAH_RETURN_EVENT_MESSAGE(_generation->type(), "Concurrent reset after collect", " (unload classes)");
+  } else {
+    SHENANDOAH_RETURN_EVENT_MESSAGE(_generation->type(), "Concurrent reset after collect", "");
+  }
+}
+
 const char* ShenandoahConcurrentGC::final_roots_event_message() const {
   if (ShenandoahHeap::heap()->unload_classes()) {
     SHENANDOAH_RETURN_EVENT_MESSAGE(_generation->type(), "Pause Final Roots", " (unload classes)");
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp
index a1837068a7cc0..e9c92c600bb37 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp
@@ -118,10 +118,14 @@ class ShenandoahConcurrentGC : public ShenandoahGC {
   void op_final_updaterefs();
   void op_final_roots();
   void op_cleanup_complete();
+  void op_reset_after_collect();
 
   // Check GC cancellation and abort concurrent GC
   bool check_cancellation_and_abort(ShenandoahDegenPoint point);
 
+  // Called when concurrent GC succeeds.
+  void entry_reset_after_collect();
+
 private:
   void start_mark();
 
@@ -134,6 +138,7 @@ class ShenandoahConcurrentGC : public ShenandoahGC {
   const char* final_roots_event_message() const;
   const char* conc_mark_event_message() const;
   const char* conc_reset_event_message() const;
+  const char* conc_reset_after_collect_event_message() const;
   const char* conc_weak_refs_event_message() const;
   const char* conc_weak_roots_event_message() const;
   const char* conc_cleanup_event_message() const;
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
index 2847d7c78ba18..0fe2ce157a6ce 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
@@ -193,7 +193,7 @@ void ShenandoahFullGC::do_it(GCCause::Cause gc_cause) {
     }
 
     // d. Reset the bitmaps for new marking
-    heap->global_generation()->reset_mark_bitmap();
+    heap->global_generation()->reset_mark_bitmap(true, false);
     assert(heap->marking_context()->is_bitmap_clear(), "sanity");
     assert(!heap->global_generation()->is_mark_complete(), "sanity");
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 2bebac05f158f..fbbff88dc8e54 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -63,29 +63,50 @@ class ShenandoahResetUpdateRegionStateClosure : public ShenandoahHeapRegionClosu
   bool is_thread_safe() override { return true; }
 };
 
-class ShenandoahResetBitmapTask : public WorkerTask {
+class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 private:
-  ShenandoahRegionIterator _regions;
-  ShenandoahGeneration* _generation;
+  ShenandoahHeap*           _heap;
+  ShenandoahMarkingContext* _ctx;
+  ShenandoahGeneration*     _generation;
+  bool const                _reset_for_current_cycle;
+  bool const                _reset_update_region_state;
 
 public:
-  ShenandoahResetBitmapTask(ShenandoahGeneration* generation) :
-    WorkerTask("Shenandoah Reset Bitmap"), _generation(generation) {}
-
-  void work(uint worker_id) {
-    ShenandoahHeap* heap = ShenandoahHeap::heap();
-    assert(!heap->is_uncommit_in_progress(), "Cannot uncommit bitmaps while resetting them.");
-    ShenandoahHeapRegion* region = _regions.next();
-    ShenandoahMarkingContext* const ctx = heap->marking_context();
-    while (region != nullptr) {
-      auto const affiliation = region->affiliation();
-      bool needs_reset = affiliation == FREE || _generation->contains(affiliation);
-      if (needs_reset && heap->is_bitmap_slice_committed(region)) {
-        ctx->clear_bitmap(region);
+  ShenandoahResetBitmapClosure(ShenandoahGeneration* generation,
+                               bool const reset_for_current_cycle,
+                               bool const reset_update_region_state) :
+    ShenandoahHeapRegionClosure(),
+    _heap(ShenandoahHeap::heap()),
+    _ctx(_heap->marking_context()),
+    _generation(generation),
+    _reset_for_current_cycle(reset_for_current_cycle),
+    _reset_update_region_state(reset_update_region_state) {}
+
+  void heap_region_do(ShenandoahHeapRegion* region) {
+    assert(!_heap->is_uncommit_in_progress(), "Cannot uncommit bitmaps while resetting them.");
+    if (region->need_bitmap_reset()) {
+      if (_heap->is_bitmap_slice_committed(region)) {
+        _ctx->clear_bitmap(region);
+        if (!_reset_for_current_cycle) {
+          region->unset_need_bitmap_reset();
+        }
       }
-      region = _regions.next();
+    }
+
+    if (_reset_for_current_cycle) {
+      region->set_need_bitmap_reset();
+    }
+
+    // Capture Top At Mark Start for this generation (typically young) and reset mark bitmap.
+    if (_reset_update_region_state && region->is_active()) {
+      // Reset live data and set TAMS optimistically. We would recheck these under the pause
+      // anyway to capture any updates that happened since now.
+      _ctx->capture_top_at_mark_start(region);
+      region->clear_live_data();
     }
   }
+
+  bool is_thread_safe() { return true; }
 };
 
 // Copy the write-version of the card-table into the read-version, clearing the
@@ -222,14 +243,14 @@ void ShenandoahGeneration::log_status(const char *msg) const {
                    byte_size_in_proper_unit(v_available),         proper_unit_for_byte_size(v_available));
 }
 
-void ShenandoahGeneration::reset_mark_bitmap() {
+void ShenandoahGeneration::reset_mark_bitmap(bool const for_current_cycle, bool update_region_state) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   heap->assert_gc_workers(heap->workers()->active_workers());
 
   set_mark_incomplete();
 
-  ShenandoahResetBitmapTask task(this);
-  heap->workers()->run_task(&task);
+  ShenandoahResetBitmapClosure closure(this, for_current_cycle, update_region_state);
+  parallel_heap_region_iterate_free(&closure);
 }
 
 // The ideal is to swap the remembered set so the safepoint effort is no more than a few pointer manipulations.
@@ -262,12 +283,7 @@ void ShenandoahGeneration::merge_write_table() {
 }
 
 void ShenandoahGeneration::prepare_gc() {
-
-  reset_mark_bitmap();
-
-  // Capture Top At Mark Start for this generation (typically young) and reset mark bitmap.
-  ShenandoahResetUpdateRegionStateClosure cl;
-  parallel_heap_region_iterate_free(&cl);
+  reset_mark_bitmap(true, true);
 }
 
 void ShenandoahGeneration::parallel_heap_region_iterate_free(ShenandoahHeapRegionClosure* cl) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
index 2f1102edbd707..a46f9d2c04526 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
@@ -159,7 +159,7 @@ class ShenandoahGeneration : public CHeapObj<mtGC>, public ShenandoahSpaceInfo {
   void log_status(const char* msg) const;
 
   // Used directly by FullGC
-  void reset_mark_bitmap();
+  void reset_mark_bitmap(bool for_current_cycle, bool update_region_state);
 
   // Used by concurrent and degenerated GC to reset remembered set.
   void swap_remembered_set();
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp
index d46b76c937652..480bfbae3c276 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp
@@ -89,6 +89,7 @@ ShenandoahHeapRegion::ShenandoahHeapRegion(HeapWord* start, size_t index, bool c
     SpaceMangler::mangle_region(MemRegion(_bottom, _end));
   }
   _recycling.unset();
+  _need_bitmap_reset = true;
 }
 
 void ShenandoahHeapRegion::report_illegal_transition(const char *method) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
index 3dc8febbad425..cc89dcdbbf484 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
@@ -266,6 +266,8 @@ class ShenandoahHeapRegion {
 
   ShenandoahSharedFlag _recycling; // Used to indicate that the region is being recycled; see try_recycle*().
 
+  bool _need_bitmap_reset;
+
 public:
   ShenandoahHeapRegion(HeapWord* start, size_t index, bool committed);
 
@@ -477,6 +479,18 @@ class ShenandoahHeapRegion {
 
   CENSUS_NOISE(void clear_youth() { _youth = 0; })
 
+  inline bool need_bitmap_reset() const {
+    return _need_bitmap_reset;
+  }
+
+  inline void set_need_bitmap_reset() {
+    _need_bitmap_reset = true;
+  }
+
+  inline void unset_need_bitmap_reset() {
+    _need_bitmap_reset = false;
+  }
+
 private:
   void decrement_humongous_waste() const;
   void do_commit();
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp
index a5d3302091c00..5ceb20cd7fb1e 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp
@@ -157,5 +157,8 @@ bool ShenandoahOldGC::collect(GCCause::Cause cause) {
     LogStream ls(lt);
     result.print_on("Old Mark", &ls);
   }
+
+  entry_reset_after_collect();
+
   return true;
 }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp
index 4c8cb8c20570d..9c1636c3dfedf 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp
@@ -50,6 +50,7 @@ class outputStream;
 
 #define SHENANDOAH_PHASE_DO(f)                                                         \
   f(conc_reset,                                     "Concurrent Reset")                \
+  f(conc_reset_after_collect,                       "Concurrent Reset After Collect")  \
   f(conc_reset_old,                                 "Concurrent Reset (OLD)")          \
   f(init_mark_gross,                                "Pause Init Mark (G)")             \
   f(init_mark,                                      "Pause Init Mark (N)")             \

From a8948cf86ea7e019b14569a005b8c847ab2c57f8 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Mon, 16 Dec 2024 15:09:22 -0800
Subject: [PATCH 02/18] Use template class for ShenandoahResetBitmapClosure

---
 .../gc/shenandoah/shenandoahConcurrentGC.cpp  |  4 +-
 .../share/gc/shenandoah/shenandoahFullGC.cpp  |  2 +-
 .../gc/shenandoah/shenandoahGeneration.cpp    | 47 ++++++++++---------
 .../gc/shenandoah/shenandoahGeneration.hpp    |  3 +-
 4 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index 3b90116b3afa0..62e11430152ac 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -1212,10 +1212,10 @@ void ShenandoahConcurrentGC::op_reset_after_collect() {
     if (!_do_old_gc_bootstrap) {
       // Only reset for young generation, bitmap for old generation must be retained,
       // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
-      heap->young_generation()->reset_mark_bitmap(false, false);
+      heap->young_generation()->reset_mark_bitmap<false, false>();
     }
   } else {
-    _generation->reset_mark_bitmap(false, false);
+    _generation->reset_mark_bitmap<false, false>();
   }
 }
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
index 0fe2ce157a6ce..96fed3259f652 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
@@ -193,7 +193,7 @@ void ShenandoahFullGC::do_it(GCCause::Cause gc_cause) {
     }
 
     // d. Reset the bitmaps for new marking
-    heap->global_generation()->reset_mark_bitmap(true, false);
+    heap->global_generation()->reset_mark_bitmap<true, false>();
     assert(heap->marking_context()->is_bitmap_clear(), "sanity");
     assert(!heap->global_generation()->is_mark_complete(), "sanity");
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index fbbff88dc8e54..42e1e789dbb52 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -63,42 +63,38 @@ class ShenandoahResetUpdateRegionStateClosure : public ShenandoahHeapRegionClosu
   bool is_thread_safe() override { return true; }
 };
 
+template <bool FOR_CURRENT_CYCLE, bool UPDATE_REGION_STATE>
 class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 private:
   ShenandoahHeap*           _heap;
   ShenandoahMarkingContext* _ctx;
   ShenandoahGeneration*     _generation;
-  bool const                _reset_for_current_cycle;
-  bool const                _reset_update_region_state;
 
 public:
-  ShenandoahResetBitmapClosure(ShenandoahGeneration* generation,
-                               bool const reset_for_current_cycle,
-                               bool const reset_update_region_state) :
+  explicit ShenandoahResetBitmapClosure(ShenandoahGeneration* generation) :
     ShenandoahHeapRegionClosure(),
     _heap(ShenandoahHeap::heap()),
     _ctx(_heap->marking_context()),
-    _generation(generation),
-    _reset_for_current_cycle(reset_for_current_cycle),
-    _reset_update_region_state(reset_update_region_state) {}
+    _generation(generation) {}
 
-  void heap_region_do(ShenandoahHeapRegion* region) {
+  void heap_region_do(ShenandoahHeapRegion* region) override {
     assert(!_heap->is_uncommit_in_progress(), "Cannot uncommit bitmaps while resetting them.");
-    if (region->need_bitmap_reset()) {
+    if (FOR_CURRENT_CYCLE) {
+      if (region->need_bitmap_reset() && _heap->is_bitmap_slice_committed(region)) {
+        _ctx->clear_bitmap(region);
+      } else {
+        region->set_need_bitmap_reset();
+      }
+    } else {
+      assert(region->need_bitmap_reset(), "The flag must be set to true in concurrent reset phase when current cycle starts");
       if (_heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
-        if (!_reset_for_current_cycle) {
-          region->unset_need_bitmap_reset();
-        }
+        region->unset_need_bitmap_reset();
       }
     }
 
-    if (_reset_for_current_cycle) {
-      region->set_need_bitmap_reset();
-    }
-
-    // Capture Top At Mark Start for this generation (typically young) and reset mark bitmap.
-    if (_reset_update_region_state && region->is_active()) {
+    // Capture Top At Mark Start for this generation (typically young).
+    if (UPDATE_REGION_STATE && region->is_active()) {
       // Reset live data and set TAMS optimistically. We would recheck these under the pause
       // anyway to capture any updates that happened since now.
       _ctx->capture_top_at_mark_start(region);
@@ -106,7 +102,7 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
     }
   }
 
-  bool is_thread_safe() { return true; }
+  bool is_thread_safe() override { return true; }
 };
 
 // Copy the write-version of the card-table into the read-version, clearing the
@@ -243,15 +239,20 @@ void ShenandoahGeneration::log_status(const char *msg) const {
                    byte_size_in_proper_unit(v_available),         proper_unit_for_byte_size(v_available));
 }
 
-void ShenandoahGeneration::reset_mark_bitmap(bool const for_current_cycle, bool update_region_state) {
+template <bool FOR_CURRENT_CYCLE, bool UPDATE_REGION_STATE>
+void ShenandoahGeneration::reset_mark_bitmap() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   heap->assert_gc_workers(heap->workers()->active_workers());
 
   set_mark_incomplete();
 
-  ShenandoahResetBitmapClosure closure(this, for_current_cycle, update_region_state);
+  ShenandoahResetBitmapClosure<FOR_CURRENT_CYCLE, UPDATE_REGION_STATE> closure(this);
   parallel_heap_region_iterate_free(&closure);
 }
+// Explicit specializations
+template void ShenandoahGeneration::reset_mark_bitmap<true, true>();
+template void ShenandoahGeneration::reset_mark_bitmap<true, false>();
+template void ShenandoahGeneration::reset_mark_bitmap<false, false>();
 
 // The ideal is to swap the remembered set so the safepoint effort is no more than a few pointer manipulations.
 // However, limitations in the implementation of the mutator write-barrier make it difficult to simply change the
@@ -283,7 +284,7 @@ void ShenandoahGeneration::merge_write_table() {
 }
 
 void ShenandoahGeneration::prepare_gc() {
-  reset_mark_bitmap(true, true);
+  reset_mark_bitmap<true, true>();
 }
 
 void ShenandoahGeneration::parallel_heap_region_iterate_free(ShenandoahHeapRegionClosure* cl) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
index a46f9d2c04526..9fb87d83b83c0 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
@@ -159,7 +159,8 @@ class ShenandoahGeneration : public CHeapObj<mtGC>, public ShenandoahSpaceInfo {
   void log_status(const char* msg) const;
 
   // Used directly by FullGC
-  void reset_mark_bitmap(bool for_current_cycle, bool update_region_state);
+  template <bool FOR_CURRENT_CYCLE, bool UPDATE_REGION_STATE>
+  void reset_mark_bitmap();
 
   // Used by concurrent and degenerated GC to reset remembered set.
   void swap_remembered_set();

From 11fdb12ebdb77f5f29f1a6a23cdf98e2ee154ec0 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Mon, 16 Dec 2024 16:07:10 -0800
Subject: [PATCH 03/18] Remove _generation from ShenandoahResetBitmapClosure

---
 .../share/gc/shenandoah/shenandoahGeneration.cpp       | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 42e1e789dbb52..88b4f28e1cc9f 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -68,14 +68,10 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 private:
   ShenandoahHeap*           _heap;
   ShenandoahMarkingContext* _ctx;
-  ShenandoahGeneration*     _generation;
 
 public:
-  explicit ShenandoahResetBitmapClosure(ShenandoahGeneration* generation) :
-    ShenandoahHeapRegionClosure(),
-    _heap(ShenandoahHeap::heap()),
-    _ctx(_heap->marking_context()),
-    _generation(generation) {}
+  explicit ShenandoahResetBitmapClosure() :
+    ShenandoahHeapRegionClosure(), _heap(ShenandoahHeap::heap()), _ctx(_heap->marking_context()) {}
 
   void heap_region_do(ShenandoahHeapRegion* region) override {
     assert(!_heap->is_uncommit_in_progress(), "Cannot uncommit bitmaps while resetting them.");
@@ -246,7 +242,7 @@ void ShenandoahGeneration::reset_mark_bitmap() {
 
   set_mark_incomplete();
 
-  ShenandoahResetBitmapClosure<FOR_CURRENT_CYCLE, UPDATE_REGION_STATE> closure(this);
+  ShenandoahResetBitmapClosure<FOR_CURRENT_CYCLE, UPDATE_REGION_STATE> closure;
   parallel_heap_region_iterate_free(&closure);
 }
 // Explicit specializations

From 859f9377ba7c55879578faef10e2a17d9ac203bc Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Mon, 16 Dec 2024 16:45:07 -0800
Subject: [PATCH 04/18] Remove assert

---
 src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 88b4f28e1cc9f..0fc1db21e79be 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -82,7 +82,6 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
         region->set_need_bitmap_reset();
       }
     } else {
-      assert(region->need_bitmap_reset(), "The flag must be set to true in concurrent reset phase when current cycle starts");
       if (_heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
         region->unset_need_bitmap_reset();

From bc278bd7b6a93268f80bcac405b592c012e4629e Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Tue, 17 Dec 2024 12:39:02 -0800
Subject: [PATCH 05/18] Remove ShenandoahPrepareForMarkClosure used by
 ShenandoahFullGC

---
 .../share/gc/shenandoah/shenandoahFullGC.cpp  | 30 ++++---------------
 .../gc/shenandoah/shenandoahGeneration.cpp    |  2 ++
 2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
index 96fed3259f652..847714b409b53 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
@@ -192,16 +192,11 @@ void ShenandoahFullGC::do_it(GCCause::Cause gc_cause) {
       update_roots(true /*full_gc*/);
     }
 
-    // d. Reset the bitmaps for new marking
-    heap->global_generation()->reset_mark_bitmap<true, false>();
-    assert(heap->marking_context()->is_bitmap_clear(), "sanity");
-    assert(!heap->global_generation()->is_mark_complete(), "sanity");
-
-    // e. Abandon reference discovery and clear all discovered references.
+    // d. Abandon reference discovery and clear all discovered references.
     ShenandoahReferenceProcessor* rp = heap->global_generation()->ref_processor();
     rp->abandon_partial_discovery();
 
-    // f. Sync pinned region status from the CP marks
+    // e. Sync pinned region status from the CP marks
     heap->sync_pinned_region_status();
 
     if (heap->mode()->is_generational()) {
@@ -282,30 +277,15 @@ void ShenandoahFullGC::do_it(GCCause::Cause gc_cause) {
   }
 }
 
-class ShenandoahPrepareForMarkClosure: public ShenandoahHeapRegionClosure {
-private:
-  ShenandoahMarkingContext* const _ctx;
-
-public:
-  ShenandoahPrepareForMarkClosure() : _ctx(ShenandoahHeap::heap()->marking_context()) {}
-
-  void heap_region_do(ShenandoahHeapRegion *r) override {
-    _ctx->capture_top_at_mark_start(r);
-    r->clear_live_data();
-  }
-
-  bool is_thread_safe() override { return true; }
-};
-
 void ShenandoahFullGC::phase1_mark_heap() {
   GCTraceTime(Info, gc, phases) time("Phase 1: Mark live objects", _gc_timer);
   ShenandoahGCPhase mark_phase(ShenandoahPhaseTimings::full_gc_mark);
 
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
-  ShenandoahPrepareForMarkClosure prepare_for_mark;
-  ShenandoahExcludeRegionClosure<FREE> cl(&prepare_for_mark);
-  heap->parallel_heap_region_iterate(&cl);
+  heap->global_generation()->reset_mark_bitmap<true, true>();
+  assert(heap->marking_context()->is_bitmap_clear(), "sanity");
+  assert(!heap->global_generation()->is_mark_complete(), "sanity");
 
   heap->set_unload_classes(heap->global_generation()->heuristics()->can_unload_classes());
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 0fc1db21e79be..0f7a39a0e55e0 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -85,6 +85,8 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
       if (_heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
         region->unset_need_bitmap_reset();
+      } else {
+        region->set_need_bitmap_reset();
       }
     }
 

From 145a0906ae2b68272d87095b421ba6044d2ea776 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Tue, 17 Dec 2024 14:13:26 -0800
Subject: [PATCH 06/18] Merge Concurrent reset (Old) into Concurrent reset

---
 .../gc/shenandoah/shenandoahConcurrentGC.cpp    | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index 62e11430152ac..b3ca90912f14b 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -363,17 +363,6 @@ void ShenandoahConcurrentGC::entry_reset() {
                                 msg);
     op_reset();
   }
-
-  if (_do_old_gc_bootstrap) {
-    static const char* msg = "Concurrent reset (Old)";
-    ShenandoahConcurrentPhase gc_phase(msg, ShenandoahPhaseTimings::conc_reset_old);
-    ShenandoahWorkerScope scope(ShenandoahHeap::heap()->workers(),
-                                ShenandoahWorkerPolicy::calc_workers_for_conc_reset(),
-                                msg);
-    EventMark em("%s", msg);
-
-    heap->old_generation()->prepare_gc();
-  }
 }
 
 void ShenandoahConcurrentGC::entry_scan_remembered_set() {
@@ -598,7 +587,11 @@ void ShenandoahConcurrentGC::op_reset() {
   if (ShenandoahPacing) {
     heap->pacer()->setup_for_reset();
   }
-  _generation->prepare_gc();
+  if (_do_old_gc_bootstrap) {
+    heap->global_generation()->prepare_gc();
+  } else {
+    _generation->prepare_gc();
+  }
 }
 
 class ShenandoahInitMarkUpdateRegionStateClosure : public ShenandoahHeapRegionClosure {

From a689932176235e92dec108e7607f164a40d522f8 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Tue, 17 Dec 2024 16:16:23 -0800
Subject: [PATCH 07/18] Renaming, comments, cleanup

---
 .../gc/shenandoah/shenandoahConcurrentGC.cpp  |  4 +--
 .../share/gc/shenandoah/shenandoahFullGC.cpp  |  2 +-
 .../gc/shenandoah/shenandoahGeneration.cpp    | 30 +++++++++----------
 .../gc/shenandoah/shenandoahGeneration.hpp    |  2 +-
 4 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index b3ca90912f14b..edc0b76c25c39 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -1205,10 +1205,10 @@ void ShenandoahConcurrentGC::op_reset_after_collect() {
     if (!_do_old_gc_bootstrap) {
       // Only reset for young generation, bitmap for old generation must be retained,
       // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
-      heap->young_generation()->reset_mark_bitmap<false, false>();
+      heap->young_generation()->reset_mark_bitmap<false>();
     }
   } else {
-    _generation->reset_mark_bitmap<false, false>();
+    _generation->reset_mark_bitmap<false>();
   }
 }
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
index 847714b409b53..b18b330d60ea0 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
@@ -283,7 +283,7 @@ void ShenandoahFullGC::phase1_mark_heap() {
 
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
-  heap->global_generation()->reset_mark_bitmap<true, true>();
+  heap->global_generation()->reset_mark_bitmap<true>();
   assert(heap->marking_context()->is_bitmap_clear(), "sanity");
   assert(!heap->global_generation()->is_mark_complete(), "sanity");
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 0f7a39a0e55e0..4b8c47017dbe6 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -63,7 +63,7 @@ class ShenandoahResetUpdateRegionStateClosure : public ShenandoahHeapRegionClosu
   bool is_thread_safe() override { return true; }
 };
 
-template <bool FOR_CURRENT_CYCLE, bool UPDATE_REGION_STATE>
+template <bool PREPARE_FOR_CURRENT_CYCLE>
 class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 private:
   ShenandoahHeap*           _heap;
@@ -75,12 +75,19 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 
   void heap_region_do(ShenandoahHeapRegion* region) override {
     assert(!_heap->is_uncommit_in_progress(), "Cannot uncommit bitmaps while resetting them.");
-    if (FOR_CURRENT_CYCLE) {
+    if (PREPARE_FOR_CURRENT_CYCLE) {
       if (region->need_bitmap_reset() && _heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
       } else {
         region->set_need_bitmap_reset();
       }
+      // Capture Top At Mark Start for this generation.
+      if (region->is_active()) {
+        // Reset live data and set TAMS optimistically. We would recheck these under the pause
+        // anyway to capture any updates that happened since now.
+        _ctx->capture_top_at_mark_start(region);
+        region->clear_live_data();
+      }
     } else {
       if (_heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
@@ -89,14 +96,6 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
         region->set_need_bitmap_reset();
       }
     }
-
-    // Capture Top At Mark Start for this generation (typically young).
-    if (UPDATE_REGION_STATE && region->is_active()) {
-      // Reset live data and set TAMS optimistically. We would recheck these under the pause
-      // anyway to capture any updates that happened since now.
-      _ctx->capture_top_at_mark_start(region);
-      region->clear_live_data();
-    }
   }
 
   bool is_thread_safe() override { return true; }
@@ -236,20 +235,19 @@ void ShenandoahGeneration::log_status(const char *msg) const {
                    byte_size_in_proper_unit(v_available),         proper_unit_for_byte_size(v_available));
 }
 
-template <bool FOR_CURRENT_CYCLE, bool UPDATE_REGION_STATE>
+template <bool PREPARE_FOR_CURRENT_CYCLE>
 void ShenandoahGeneration::reset_mark_bitmap() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   heap->assert_gc_workers(heap->workers()->active_workers());
 
   set_mark_incomplete();
 
-  ShenandoahResetBitmapClosure<FOR_CURRENT_CYCLE, UPDATE_REGION_STATE> closure;
+  ShenandoahResetBitmapClosure<PREPARE_FOR_CURRENT_CYCLE> closure;
   parallel_heap_region_iterate_free(&closure);
 }
 // Explicit specializations
-template void ShenandoahGeneration::reset_mark_bitmap<true, true>();
-template void ShenandoahGeneration::reset_mark_bitmap<true, false>();
-template void ShenandoahGeneration::reset_mark_bitmap<false, false>();
+template void ShenandoahGeneration::reset_mark_bitmap<true>();
+template void ShenandoahGeneration::reset_mark_bitmap<false>();
 
 // The ideal is to swap the remembered set so the safepoint effort is no more than a few pointer manipulations.
 // However, limitations in the implementation of the mutator write-barrier make it difficult to simply change the
@@ -281,7 +279,7 @@ void ShenandoahGeneration::merge_write_table() {
 }
 
 void ShenandoahGeneration::prepare_gc() {
-  reset_mark_bitmap<true, true>();
+  reset_mark_bitmap<true>();
 }
 
 void ShenandoahGeneration::parallel_heap_region_iterate_free(ShenandoahHeapRegionClosure* cl) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
index 9fb87d83b83c0..1e8afe5685235 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
@@ -159,7 +159,7 @@ class ShenandoahGeneration : public CHeapObj<mtGC>, public ShenandoahSpaceInfo {
   void log_status(const char* msg) const;
 
   // Used directly by FullGC
-  template <bool FOR_CURRENT_CYCLE, bool UPDATE_REGION_STATE>
+  template <bool FOR_CURRENT_CYCLE>
   void reset_mark_bitmap();
 
   // Used by concurrent and degenerated GC to reset remembered set.

From b43b10d1283f5cbef3ff8c63a59a6940ad663197 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Wed, 18 Dec 2024 13:30:05 -0800
Subject: [PATCH 08/18] Not invoke set_mark_incomplete when reset bitmap after
 cycle

---
 src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 4b8c47017dbe6..00ad25914319a 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -240,7 +240,9 @@ void ShenandoahGeneration::reset_mark_bitmap() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   heap->assert_gc_workers(heap->workers()->active_workers());
 
-  set_mark_incomplete();
+  if (PREPARE_FOR_CURRENT_CYCLE) {
+    set_mark_incomplete();
+  }
 
   ShenandoahResetBitmapClosure<PREPARE_FOR_CURRENT_CYCLE> closure;
   parallel_heap_region_iterate_free(&closure);

From 450ab1729845d0ecad87ce21815880e08239a0e6 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 19 Dec 2024 16:25:32 -0800
Subject: [PATCH 09/18] Not reset_mark_bitmap after cycle when 
 is_concurrent_old_mark_in_progress or is_prepare_for_old_mark_in_progress

---
 src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index edc0b76c25c39..b11d09ba9a044 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -1202,7 +1202,9 @@ void ShenandoahConcurrentGC::op_reset_after_collect() {
 
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
   if (heap->mode()->is_generational()) {
-    if (!_do_old_gc_bootstrap) {
+    if (!_do_old_gc_bootstrap &&
+        !heap->is_concurrent_old_mark_in_progress() &&
+        !heap->is_prepare_for_old_mark_in_progress()) {
       // Only reset for young generation, bitmap for old generation must be retained,
       // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
       heap->young_generation()->reset_mark_bitmap<false>();

From ae6843e7e3097366d47fd98922d54b6a1c01d48f Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 19 Dec 2024 18:36:52 -0800
Subject: [PATCH 10/18] fix

---
 src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index b11d09ba9a044..354c2cf697262 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -1202,9 +1202,7 @@ void ShenandoahConcurrentGC::op_reset_after_collect() {
 
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
   if (heap->mode()->is_generational()) {
-    if (!_do_old_gc_bootstrap &&
-        !heap->is_concurrent_old_mark_in_progress() &&
-        !heap->is_prepare_for_old_mark_in_progress()) {
+    if (!_do_old_gc_bootstrap && !heap->is_concurrent_old_mark_in_progress()) {
       // Only reset for young generation, bitmap for old generation must be retained,
       // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
       heap->young_generation()->reset_mark_bitmap<false>();

From f71d7d557ac948c39d2f567a8d1563f4205c1ac2 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 19 Dec 2024 19:46:30 -0800
Subject: [PATCH 11/18] Add comments

---
 src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index 354c2cf697262..abe93342c18ef 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -587,7 +587,11 @@ void ShenandoahConcurrentGC::op_reset() {
   if (ShenandoahPacing) {
     heap->pacer()->setup_for_reset();
   }
-  if (_do_old_gc_bootstrap) {
+  // If it old GC bootstrap cycle, or there was attempt to bootstrap old GC but get canclled after
+  // concurrent old mark in progress is set, always clear bitmap for global gen to ensure bitmap for old gen
+  // is clear for old marking after this cycle.
+  if (_do_old_gc_bootstrap || heap->is_concurrent_old_mark_in_progress()) {
+    assert(!heap->is_prepare_for_old_mark_in_progress(), "Cannot reset old without making it parsable");
     heap->global_generation()->prepare_gc();
   } else {
     _generation->prepare_gc();

From 022ac7bd2b159dafe26faedaaf3bd14eadaf919a Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 19 Dec 2024 21:30:45 -0800
Subject: [PATCH 12/18] Fix

---
 src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index abe93342c18ef..3e499a54f44db 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -587,10 +587,9 @@ void ShenandoahConcurrentGC::op_reset() {
   if (ShenandoahPacing) {
     heap->pacer()->setup_for_reset();
   }
-  // If it old GC bootstrap cycle, or there was attempt to bootstrap old GC but get canclled after
-  // concurrent old mark in progress is set, always clear bitmap for global gen to ensure bitmap for old gen
-  // is clear for old marking after this cycle.
-  if (_do_old_gc_bootstrap || heap->is_concurrent_old_mark_in_progress()) {
+  // If it is old GC bootstrap cycle, always clear bitmap for global gen
+  // to ensure bitmap for old gen is clear for old GC cycle after this.
+  if (_do_old_gc_bootstrap) {
     assert(!heap->is_prepare_for_old_mark_in_progress(), "Cannot reset old without making it parsable");
     heap->global_generation()->prepare_gc();
   } else {

From cf9e0a578f459aeec56b451e91a9b58e7104d035 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 19 Dec 2024 21:34:35 -0800
Subject: [PATCH 13/18] Always set_mark_incomplete when reset mark bitmap

---
 src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 00ad25914319a..4b8c47017dbe6 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -240,9 +240,7 @@ void ShenandoahGeneration::reset_mark_bitmap() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   heap->assert_gc_workers(heap->workers()->active_workers());
 
-  if (PREPARE_FOR_CURRENT_CYCLE) {
-    set_mark_incomplete();
-  }
+  set_mark_incomplete();
 
   ShenandoahResetBitmapClosure<PREPARE_FOR_CURRENT_CYCLE> closure;
   parallel_heap_region_iterate_free(&closure);

From 1f748c060cccff956e41e85f8ad2d2f9234c29b2 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 19 Dec 2024 21:57:51 -0800
Subject: [PATCH 14/18] Remove ShenandoahResetUpdateRegionStateClosure

---
 .../gc/shenandoah/shenandoahGeneration.cpp    | 22 -------------------
 1 file changed, 22 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index 4b8c47017dbe6..dfda95b536809 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -41,28 +41,6 @@
 
 #include "utilities/quickSort.hpp"
 
-
-class ShenandoahResetUpdateRegionStateClosure : public ShenandoahHeapRegionClosure {
-private:
-  ShenandoahHeap* _heap;
-  ShenandoahMarkingContext* const _ctx;
-public:
-  ShenandoahResetUpdateRegionStateClosure() :
-    _heap(ShenandoahHeap::heap()),
-    _ctx(_heap->marking_context()) {}
-
-  void heap_region_do(ShenandoahHeapRegion* r) override {
-    if (r->is_active()) {
-      // Reset live data and set TAMS optimistically. We would recheck these under the pause
-      // anyway to capture any updates that happened since now.
-      _ctx->capture_top_at_mark_start(r);
-      r->clear_live_data();
-    }
-  }
-
-  bool is_thread_safe() override { return true; }
-};
-
 template <bool PREPARE_FOR_CURRENT_CYCLE>
 class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 private:

From 36f14832ccca187f65e85a8d175dfe513df1c716 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Tue, 24 Dec 2024 23:34:26 -0800
Subject: [PATCH 15/18] Address review comments

---
 .../share/gc/shenandoah/shenandoahFullGC.cpp  |  2 +-
 .../gc/shenandoah/shenandoahGeneration.cpp    | 19 ++++++++++---------
 .../gc/shenandoah/shenandoahGeneration.hpp    |  2 +-
 .../gc/shenandoah/shenandoahHeapRegion.cpp    |  6 +++---
 .../gc/shenandoah/shenandoahHeapRegion.hpp    | 12 ++++++------
 5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
index b18b330d60ea0..847714b409b53 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp
@@ -283,7 +283,7 @@ void ShenandoahFullGC::phase1_mark_heap() {
 
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
-  heap->global_generation()->reset_mark_bitmap<true>();
+  heap->global_generation()->reset_mark_bitmap<true, true>();
   assert(heap->marking_context()->is_bitmap_clear(), "sanity");
   assert(!heap->global_generation()->is_mark_complete(), "sanity");
 
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
index dfda95b536809..0d4fcb4e13f31 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp
@@ -41,7 +41,7 @@
 
 #include "utilities/quickSort.hpp"
 
-template <bool PREPARE_FOR_CURRENT_CYCLE>
+template <bool PREPARE_FOR_CURRENT_CYCLE, bool FULL_GC = false>
 class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
 private:
   ShenandoahHeap*           _heap;
@@ -57,10 +57,10 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
       if (region->need_bitmap_reset() && _heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
       } else {
-        region->set_need_bitmap_reset();
+        region->set_needs_bitmap_reset();
       }
       // Capture Top At Mark Start for this generation.
-      if (region->is_active()) {
+      if (FULL_GC || region->is_active()) {
         // Reset live data and set TAMS optimistically. We would recheck these under the pause
         // anyway to capture any updates that happened since now.
         _ctx->capture_top_at_mark_start(region);
@@ -69,9 +69,9 @@ class ShenandoahResetBitmapClosure final : public ShenandoahHeapRegionClosure {
     } else {
       if (_heap->is_bitmap_slice_committed(region)) {
         _ctx->clear_bitmap(region);
-        region->unset_need_bitmap_reset();
+        region->unset_needs_bitmap_reset();
       } else {
-        region->set_need_bitmap_reset();
+        region->set_needs_bitmap_reset();
       }
     }
   }
@@ -213,19 +213,20 @@ void ShenandoahGeneration::log_status(const char *msg) const {
                    byte_size_in_proper_unit(v_available),         proper_unit_for_byte_size(v_available));
 }
 
-template <bool PREPARE_FOR_CURRENT_CYCLE>
+template <bool PREPARE_FOR_CURRENT_CYCLE, bool FULL_GC>
 void ShenandoahGeneration::reset_mark_bitmap() {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   heap->assert_gc_workers(heap->workers()->active_workers());
 
   set_mark_incomplete();
 
-  ShenandoahResetBitmapClosure<PREPARE_FOR_CURRENT_CYCLE> closure;
+  ShenandoahResetBitmapClosure<PREPARE_FOR_CURRENT_CYCLE, FULL_GC> closure;
   parallel_heap_region_iterate_free(&closure);
 }
 // Explicit specializations
-template void ShenandoahGeneration::reset_mark_bitmap<true>();
-template void ShenandoahGeneration::reset_mark_bitmap<false>();
+template void ShenandoahGeneration::reset_mark_bitmap<true, false>();
+template void ShenandoahGeneration::reset_mark_bitmap<true, true>();
+template void ShenandoahGeneration::reset_mark_bitmap<false, false>();
 
 // The ideal is to swap the remembered set so the safepoint effort is no more than a few pointer manipulations.
 // However, limitations in the implementation of the mutator write-barrier make it difficult to simply change the
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
index 1e8afe5685235..1dc777777aa67 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp
@@ -159,7 +159,7 @@ class ShenandoahGeneration : public CHeapObj<mtGC>, public ShenandoahSpaceInfo {
   void log_status(const char* msg) const;
 
   // Used directly by FullGC
-  template <bool FOR_CURRENT_CYCLE>
+  template <bool FOR_CURRENT_CYCLE, bool FULL_GC = false>
   void reset_mark_bitmap();
 
   // Used by concurrent and degenerated GC to reset remembered set.
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp
index 480bfbae3c276..cd0ec04327bdf 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp
@@ -77,10 +77,11 @@ ShenandoahHeapRegion::ShenandoahHeapRegion(HeapWord* start, size_t index, bool c
   _live_data(0),
   _critical_pins(0),
   _update_watermark(start),
-  _age(0)
+  _age(0),
 #ifdef SHENANDOAH_CENSUS_NOISE
-  , _youth(0)
+  _youth(0),
 #endif // SHENANDOAH_CENSUS_NOISE
+  _needs_bitmap_reset(false)
   {
 
   assert(Universe::on_page_boundary(_bottom) && Universe::on_page_boundary(_end),
@@ -89,7 +90,6 @@ ShenandoahHeapRegion::ShenandoahHeapRegion(HeapWord* start, size_t index, bool c
     SpaceMangler::mangle_region(MemRegion(_bottom, _end));
   }
   _recycling.unset();
-  _need_bitmap_reset = true;
 }
 
 void ShenandoahHeapRegion::report_illegal_transition(const char *method) {
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
index cc89dcdbbf484..4c99364bc6ed4 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
@@ -266,7 +266,7 @@ class ShenandoahHeapRegion {
 
   ShenandoahSharedFlag _recycling; // Used to indicate that the region is being recycled; see try_recycle*().
 
-  bool _need_bitmap_reset;
+  bool _needs_bitmap_reset;
 
 public:
   ShenandoahHeapRegion(HeapWord* start, size_t index, bool committed);
@@ -480,15 +480,15 @@ class ShenandoahHeapRegion {
   CENSUS_NOISE(void clear_youth() { _youth = 0; })
 
   inline bool need_bitmap_reset() const {
-    return _need_bitmap_reset;
+    return _needs_bitmap_reset;
   }
 
-  inline void set_need_bitmap_reset() {
-    _need_bitmap_reset = true;
+  inline void set_needs_bitmap_reset() {
+    _needs_bitmap_reset = true;
   }
 
-  inline void unset_need_bitmap_reset() {
-    _need_bitmap_reset = false;
+  inline void unset_needs_bitmap_reset() {
+    _needs_bitmap_reset = false;
   }
 
 private:

From 4ae220fbefadf8476cee1efa9ec8827952447895 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Wed, 8 Jan 2025 14:59:00 -0800
Subject: [PATCH 16/18] Remove condition check !_do_old_gc_bootstrap &&
 !heap->is_concurrent_old_mark_in_progress() from op_reset_after_collect

---
 src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index 3e499a54f44db..52c49c376ba74 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -1205,11 +1205,11 @@ void ShenandoahConcurrentGC::op_reset_after_collect() {
 
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
   if (heap->mode()->is_generational()) {
-    if (!_do_old_gc_bootstrap && !heap->is_concurrent_old_mark_in_progress()) {
+    //if (!_do_old_gc_bootstrap && !heap->is_concurrent_old_mark_in_progress()) {
       // Only reset for young generation, bitmap for old generation must be retained,
       // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
       heap->young_generation()->reset_mark_bitmap<false>();
-    }
+    //}
   } else {
     _generation->reset_mark_bitmap<false>();
   }

From 6b4a892fbe8b2b13a28780478fd20d300dadf5ea Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Wed, 8 Jan 2025 15:14:36 -0800
Subject: [PATCH 17/18] Remove entry_reset_after_collect from ShenandoahOldGC

---
 src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp
index 5ceb20cd7fb1e..a5d3302091c00 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahOldGC.cpp
@@ -157,8 +157,5 @@ bool ShenandoahOldGC::collect(GCCause::Cause cause) {
     LogStream ls(lt);
     result.print_on("Old Mark", &ls);
   }
-
-  entry_reset_after_collect();
-
   return true;
 }

From 04299a7697085bfe51dd01198f0e72cfde228cf6 Mon Sep 17 00:00:00 2001
From: Xiaolong Peng <xlpeng@amazon.com>
Date: Thu, 9 Jan 2025 11:23:56 -0800
Subject: [PATCH 18/18] Adding condition "!_do_old_gc_bootstrap &&
 !heap->is_concurrent_old_mark_in_progress()" back and address some PR
 comments

---
 .../gc/shenandoah/shenandoahConcurrentGC.cpp     | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
index 52c49c376ba74..0b43dd9662c47 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp
@@ -237,8 +237,9 @@ bool ShenandoahConcurrentGC::collect(GCCause::Cause cause) {
     ShenandoahGenerationalHeap::heap()->complete_concurrent_cycle();
   }
 
-  // Instead of always reset before collect, some reset can be done after collect to save
-  // the time before before the cycle so the cycle can be started as soon as possible.
+  // Instead of always resetting immediately before the start of a new GC, we can often reset at the end of the
+  // previous GC. This allows us to start the next GC cycle more quickly after a trigger condition is detected,
+  // reducing the likelihood that GC will degenerate.
   entry_reset_after_collect();
 
   return true;
@@ -1205,11 +1206,14 @@ void ShenandoahConcurrentGC::op_reset_after_collect() {
 
   ShenandoahHeap* const heap = ShenandoahHeap::heap();
   if (heap->mode()->is_generational()) {
-    //if (!_do_old_gc_bootstrap && !heap->is_concurrent_old_mark_in_progress()) {
-      // Only reset for young generation, bitmap for old generation must be retained,
-      // except there is collection(global/old/degen/full) trigged to collect regions in old gen.
+    // Resetting bitmaps of young gen when bootstrap old GC or there is preempted old GC
+    // causes crash due to remembered set violation, hence condition is added to fix the crash.
+    // Assuming bitmaps of young gen are not used at all after the cycle, the crash should not
+    // have happend, it seems to tickle a bug in remembered set scan. Root causing and fixing of the bug
+    // will be tracked via ticket https://bugs.openjdk.org/browse/JDK-8347371
+    if (!_do_old_gc_bootstrap && !heap->is_concurrent_old_mark_in_progress()) {
       heap->young_generation()->reset_mark_bitmap<false>();
-    //}
+    }
   } else {
     _generation->reset_mark_bitmap<false>();
   }