8240749: Shenandoah: refactor ShenandoahUtils

Reviewed-by: rkennke

src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp

@@ -311,17 +311,14 @@ class ShenandoahUpdateThreadRootsTask : public AbstractGangTask {
 private:
   ShenandoahThreadRoots           _thread_roots;
   ShenandoahPhaseTimings::Phase   _phase;
+  ShenandoahGCWorkerPhase         _worker_phase;
 public:
   ShenandoahUpdateThreadRootsTask(bool is_par, ShenandoahPhaseTimings::Phase phase) :
     AbstractGangTask("Shenandoah Update Thread Roots"),
     _thread_roots(is_par),
-    _phase(phase) {
+    _phase(phase),
-    ShenandoahHeap::heap()->phase_timings()->record_workers_start(_phase);
+    _worker_phase(phase) {}
-  }
 
-  ~ShenandoahUpdateThreadRootsTask() {
-    ShenandoahHeap::heap()->phase_timings()->record_workers_end(_phase);
-  }
   void work(uint worker_id) {
     ShenandoahUpdateRefsClosure cl;
     _thread_roots.oops_do(&cl, NULL, worker_id);

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -2176,9 +2176,10 @@ void ShenandoahHeap::stw_process_weak_roots(bool full_gc) {
   ShenandoahPhaseTimings::Phase timing_phase = full_gc ?
                                                ShenandoahPhaseTimings::full_gc_purge_par :
                                                ShenandoahPhaseTimings::purge_par;
-  // Cleanup weak roots
   ShenandoahGCPhase phase(timing_phase);
-  phase_timings()->record_workers_start(timing_phase);
+  ShenandoahGCWorkerPhase worker_phase(timing_phase);
+
+  // Cleanup weak roots
   if (has_forwarded_objects()) {
     ShenandoahForwardedIsAliveClosure is_alive;
     ShenandoahUpdateRefsClosure keep_alive;
@@ -2197,7 +2198,6 @@ void ShenandoahHeap::stw_process_weak_roots(bool full_gc) {
 #endif
     _workers->run_task(&cleaning_task);
   }
-  phase_timings()->record_workers_end(timing_phase);
 }
 
 void ShenandoahHeap::parallel_cleaning(bool full_gc) {

src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp

@@ -174,7 +174,8 @@ public:
 
   enum Phase {
     SHENANDOAH_GC_PHASE_DO(GC_PHASE_DECLARE_ENUM)
-    _num_phases
+    _num_phases,
+    _invalid_phase = _num_phases
   };
 
   enum GCParPhases {

src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp

@@ -174,14 +174,9 @@ void ShenandoahConcurrentStringDedupRoots::oops_do(BoolObjectClosure* is_alive,
 
 ShenandoahRootProcessor::ShenandoahRootProcessor(ShenandoahPhaseTimings::Phase phase) :
   _heap(ShenandoahHeap::heap()),
-  _phase(phase) {
+  _phase(phase),
+  _worker_phase(phase) {
   assert(SafepointSynchronize::is_at_safepoint(), "Must at safepoint");
-  _heap->phase_timings()->record_workers_start(_phase);
-}
-
-ShenandoahRootProcessor::~ShenandoahRootProcessor() {
-  assert(SafepointSynchronize::is_at_safepoint(), "Must at safepoint");
-  _heap->phase_timings()->record_workers_end(_phase);
 }
 
 ShenandoahRootEvacuator::ShenandoahRootEvacuator(uint n_workers,

src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp

@@ -31,6 +31,7 @@
 #include "gc/shenandoah/shenandoahHeap.hpp"
 #include "gc/shenandoah/shenandoahPhaseTimings.hpp"
 #include "gc/shenandoah/shenandoahSharedVariables.hpp"
+#include "gc/shenandoah/shenandoahUtils.hpp"
 #include "memory/iterator.hpp"
 
 class ShenandoahSerialRoot {
@@ -221,9 +222,9 @@ class ShenandoahRootProcessor : public StackObj {
 private:
   ShenandoahHeap* const               _heap;
   const ShenandoahPhaseTimings::Phase _phase;
+  const ShenandoahGCWorkerPhase       _worker_phase;
 public:
   ShenandoahRootProcessor(ShenandoahPhaseTimings::Phase phase);
-  ~ShenandoahRootProcessor();
 
   ShenandoahHeap* heap() const { return _heap; }
 };

src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp

@@ -37,14 +37,13 @@
 #include "gc/shenandoah/shenandoahUtils.hpp"
 #include "utilities/debug.hpp"
 
-ShenandoahPhaseTimings::Phase ShenandoahGCPhase::_current_phase = ShenandoahGCPhase::_invalid_phase;
+ShenandoahPhaseTimings::Phase ShenandoahGCPhase::_current_phase = ShenandoahPhaseTimings::_invalid_phase;
 
 ShenandoahGCSession::ShenandoahGCSession(GCCause::Cause cause) :
   _heap(ShenandoahHeap::heap()),
   _timer(_heap->gc_timer()),
   _tracer(_heap->tracer()) {
-  assert(!ShenandoahGCPhase::is_valid_phase(ShenandoahGCPhase::current_phase()),
+  assert(!ShenandoahGCPhase::is_current_phase_valid(), "No current GC phase");
-    "No current GC phase");
 
   _heap->set_gc_cause(cause);
   _timer->register_gc_start();
@@ -70,8 +69,7 @@ ShenandoahGCSession::~ShenandoahGCSession() {
   _timer->register_gc_end();
   _heap->trace_heap(GCWhen::AfterGC, _tracer);
   _tracer->report_gc_end(_timer->gc_end(), _timer->time_partitions());
-  assert(!ShenandoahGCPhase::is_valid_phase(ShenandoahGCPhase::current_phase()),
+  assert(!ShenandoahGCPhase::is_current_phase_valid(), "No current GC phase");
-    "No current GC phase");
   _heap->set_gc_cause(GCCause::_no_gc);
 }
 
@@ -101,8 +99,8 @@ ShenandoahGCPauseMark::~ShenandoahGCPauseMark() {
 }
 
 ShenandoahGCPhase::ShenandoahGCPhase(const ShenandoahPhaseTimings::Phase phase) :
-  _heap(ShenandoahHeap::heap()), _phase(phase) {
+  _timings(ShenandoahHeap::heap()->phase_timings()), _phase(phase) {
-   assert(!Thread::current()->is_Worker_thread() &&
+  assert(!Thread::current()->is_Worker_thread() &&
               (Thread::current()->is_VM_thread() ||
                Thread::current()->is_ConcurrentGC_thread()),
           "Must be set by these threads");
@@ -112,12 +110,12 @@ ShenandoahGCPhase::ShenandoahGCPhase(const ShenandoahPhaseTimings::Phase phase)
 }
 
 ShenandoahGCPhase::~ShenandoahGCPhase() {
-  _heap->phase_timings()->record_phase_time(_phase, os::elapsedTime() - _start);
+  _timings->record_phase_time(_phase, os::elapsedTime() - _start);
   _current_phase = _parent_phase;
 }
 
-bool ShenandoahGCPhase::is_valid_phase(ShenandoahPhaseTimings::Phase phase) {
+bool ShenandoahGCPhase::is_current_phase_valid() {
-  return phase >= 0 && phase < ShenandoahPhaseTimings::_num_phases;
+  return _current_phase < ShenandoahPhaseTimings::_num_phases;
 }
 
 bool ShenandoahGCPhase::is_root_work_phase() {
@@ -137,6 +135,15 @@ bool ShenandoahGCPhase::is_root_work_phase() {
   }
 }
 
+ShenandoahGCWorkerPhase::ShenandoahGCWorkerPhase(const ShenandoahPhaseTimings::Phase phase) :
+    _timings(ShenandoahHeap::heap()->phase_timings()), _phase(phase) {
+  _timings->record_workers_start(_phase);
+}
+
+ShenandoahGCWorkerPhase::~ShenandoahGCWorkerPhase() {
+  _timings->record_workers_end(_phase);
+}
+
 ShenandoahWorkerSession::ShenandoahWorkerSession(uint worker_id) : _worker_id(worker_id) {
   Thread* thr = Thread::current();
   assert(ShenandoahThreadLocalData::worker_id(thr) == ShenandoahThreadLocalData::INVALID_WORKER_ID, "Already set");

src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp

@@ -56,10 +56,9 @@ public:
 
 class ShenandoahGCPhase : public StackObj {
 private:
-  static const ShenandoahPhaseTimings::Phase _invalid_phase = ShenandoahPhaseTimings::_num_phases;
+  static ShenandoahPhaseTimings::Phase  _current_phase;
-  static ShenandoahPhaseTimings::Phase       _current_phase;
 
-  ShenandoahHeap* const _heap;
+  ShenandoahPhaseTimings* const         _timings;
   const ShenandoahPhaseTimings::Phase   _phase;
   ShenandoahPhaseTimings::Phase         _parent_phase;
   double _start;
@@ -70,11 +69,19 @@ public:
 
   static ShenandoahPhaseTimings::Phase current_phase() { return _current_phase; }
 
-  static bool is_valid_phase(ShenandoahPhaseTimings::Phase phase);
+  static bool is_current_phase_valid();
-  static bool is_current_phase_valid() { return is_valid_phase(current_phase()); }
   static bool is_root_work_phase();
 };
 
+class ShenandoahGCWorkerPhase : public StackObj {
+private:
+  ShenandoahPhaseTimings* const       _timings;
+  const ShenandoahPhaseTimings::Phase _phase;
+public:
+  ShenandoahGCWorkerPhase(ShenandoahPhaseTimings::Phase phase);
+  ~ShenandoahGCWorkerPhase();
+};
+
 // Aggregates all the things that should happen before/after the pause.
 class ShenandoahGCPauseMark : public StackObj {
 private: