8240216: Shenandoah: remove ShenandoahTerminationTrace

shipilev · shipilev · commit 6ad1db40d8f7 · 2020-02-28T17:59:51.000+01:00
Reviewed-by: zgu
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
@@ -404,7 +404,6 @@ void ShenandoahConcurrentMark::mark_from_roots() {
   task_queues()->reserve(nworkers);
 
   {
-    ShenandoahTerminationTracker term(ShenandoahPhaseTimings::conc_termination);
     TaskTerminator terminator(nworkers, task_queues());
     ShenandoahConcurrentMarkingTask task(this, &terminator);
     workers->run_task(&task);
@@ -435,10 +434,6 @@ void ShenandoahConcurrentMark::finish_mark_from_roots(bool full_gc) {
     ShenandoahIsAliveSelector is_alive;
     ReferenceProcessorIsAliveMutator fix_isalive(_heap->ref_processor(), is_alive.is_alive_closure());
 
-    ShenandoahTerminationTracker termination_tracker(full_gc ?
-                                                     ShenandoahPhaseTimings::full_gc_mark_termination :
-                                                     ShenandoahPhaseTimings::termination);
-
     StrongRootsScope scope(nworkers);
     TaskTerminator terminator(nworkers, task_queues());
     ShenandoahFinalMarkingTask task(this, &terminator, ShenandoahStringDedup::is_enabled());
@@ -636,11 +631,6 @@ void ShenandoahConcurrentMark::weak_refs_work_doit(bool full_gc) {
           ShenandoahPhaseTimings::full_gc_weakrefs_process :
           ShenandoahPhaseTimings::weakrefs_process;
 
-  ShenandoahPhaseTimings::Phase phase_process_termination =
-          full_gc ?
-          ShenandoahPhaseTimings::full_gc_weakrefs_termination :
-          ShenandoahPhaseTimings::weakrefs_termination;
-
   shenandoah_assert_rp_isalive_not_installed();
   ShenandoahIsAliveSelector is_alive;
   ReferenceProcessorIsAliveMutator fix_isalive(rp, is_alive.is_alive_closure());
@@ -667,7 +657,6 @@ void ShenandoahConcurrentMark::weak_refs_work_doit(bool full_gc) {
 
   {
     ShenandoahGCPhase phase(phase_process);
-    ShenandoahTerminationTracker phase_term(phase_process_termination);
 
     if (_heap->has_forwarded_objects()) {
       ShenandoahCMKeepAliveUpdateClosure keep_alive(get_queue(serial_worker_id));
@@ -942,7 +931,6 @@ void ShenandoahConcurrentMark::mark_loop_work(T* cl, jushort* live_data, uint wo
       // No work encountered in current stride, try to terminate.
       // Need to leave the STS here otherwise it might block safepoints.
       ShenandoahSuspendibleThreadSetLeaver stsl(CANCELLABLE && ShenandoahSuspendibleWorkers);
-      ShenandoahTerminationTimingsTracker term_tracker(worker_id);
       ShenandoahTerminatorTerminator tt(heap);
       if (terminator->offer_termination(&tt)) return;
     }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp
@@ -44,7 +44,6 @@ const char* ShenandoahPhaseTimings::_phase_names[] = {
 ShenandoahPhaseTimings::ShenandoahPhaseTimings() : _policy(NULL) {
   uint max_workers = MAX2(ConcGCThreads, ParallelGCThreads);
   _worker_times = new ShenandoahWorkerTimings(max_workers);
-  _termination_times = new ShenandoahTerminationTimings(max_workers);
   _policy = ShenandoahHeap::heap()->shenandoah_policy();
   assert(_policy != NULL, "Can not be NULL");
 }
@@ -165,30 +164,3 @@ void ShenandoahWorkerTimings::print() const {
     _gc_par_phases[i]->print_summary_on(tty);
   }
 }
-
-
-ShenandoahTerminationTimings::ShenandoahTerminationTimings(uint max_gc_threads) {
-  _gc_termination_phase = new WorkerDataArray<double>("Task Termination (ms):", max_gc_threads);
-}
-
-void ShenandoahTerminationTimings::record_time_secs(uint worker_id, double secs) {
-  if (_gc_termination_phase->get(worker_id) == WorkerDataArray<double>::uninitialized()) {
-    _gc_termination_phase->set(worker_id, secs);
-  } else {
-    // worker may re-enter termination phase
-    _gc_termination_phase->add(worker_id, secs);
-  }
-}
-
-void ShenandoahTerminationTimings::print() const {
-  _gc_termination_phase->print_summary_on(tty);
-}
-
-double ShenandoahTerminationTimings::average() const {
-  return _gc_termination_phase->average();
-}
-
-void ShenandoahTerminationTimings::reset() {
-  _gc_termination_phase->reset();
-}
-
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp
@@ -31,7 +31,6 @@
 
 class ShenandoahCollectorPolicy;
 class ShenandoahWorkerTimings;
-class ShenandoahTerminationTimings;
 class outputStream;
 
 #define SHENANDOAH_GC_PHASE_DO(f)                                                       \
@@ -92,10 +91,8 @@ class outputStream;
   f(update_finish_queues,                           "    U: Finish Queues")             \
                                                                                         \
   f(finish_queues,                                  "  Finish Queues")                  \
-  f(termination,                                    "    Termination")                  \
   f(weakrefs,                                       "  Weak References")                \
   f(weakrefs_process,                               "    Process")                      \
-  f(weakrefs_termination,                           "      Termination")                \
   f(purge,                                          "  System Purge")                   \
   f(purge_class_unload,                             "    Unload Classes")               \
   f(purge_par,                                      "    Parallel Cleanup")             \
@@ -244,7 +241,6 @@ class outputStream;
   f(final_traversal_gc_string_dedup_table_roots,     "    TF: Dedup Table Roots")       \
   f(final_traversal_gc_string_dedup_queue_roots,     "    TF: Dedup Queue Roots")       \
   f(final_traversal_gc_finish_queues,                "    TF: Finish Queues")           \
-  f(final_traversal_gc_termination,                  "    TF:   Termination")           \
                                                                                         \
   /* Per-thread timer block, should have "roots" counters in consistent order */        \
   f(final_traversal_update_roots,                       "  Update Roots")               \
@@ -300,10 +296,8 @@ class outputStream;
                                                                                         \
   f(full_gc_mark,                                    "  Mark")                          \
   f(full_gc_mark_finish_queues,                      "    Finish Queues")               \
-  f(full_gc_mark_termination,                        "      Termination")               \
   f(full_gc_weakrefs,                                "    Weak References")             \
   f(full_gc_weakrefs_process,                        "      Process")                   \
-  f(full_gc_weakrefs_termination,                    "        Termination")             \
   f(full_gc_purge,                                   "    System Purge")                \
   f(full_gc_purge_class_unload,                      "      Unload Classes")            \
   f(full_gc_purge_par,                               "    Parallel Cleanup")            \
@@ -322,14 +316,12 @@ class outputStream;
   /* Longer concurrent phases at the end */                                             \
   f(conc_reset,                                      "Concurrent Reset")                \
   f(conc_mark,                                       "Concurrent Marking")              \
-  f(conc_termination,                                "  Termination")                   \
   f(conc_preclean,                                   "Concurrent Precleaning")          \
   f(conc_roots,                                      "Concurrent Roots")                \
   f(conc_evac,                                       "Concurrent Evacuation")           \
   f(conc_update_refs,                                "Concurrent Update Refs")          \
   f(conc_cleanup,                                    "Concurrent Cleanup")              \
   f(conc_traversal,                                  "Concurrent Traversal")            \
-  f(conc_traversal_termination,                      "  Termination")                   \
                                                                                         \
   f(conc_uncommit,                                   "Concurrent Uncommit")             \
                                                                                         \
@@ -390,15 +382,13 @@ class ShenandoahPhaseTimings : public CHeapObj<mtGC> {
   static const char*  _phase_names[_num_phases];
 
   ShenandoahWorkerTimings*      _worker_times;
-  ShenandoahTerminationTimings* _termination_times;
 
   ShenandoahCollectorPolicy* _policy;
 
 public:
   ShenandoahPhaseTimings();
 
   ShenandoahWorkerTimings* const worker_times() const { return _worker_times; }
-  ShenandoahTerminationTimings* const termination_times() const { return _termination_times; }
 
   // record phase start
   void record_phase_start(Phase phase);
@@ -438,19 +428,4 @@ class ShenandoahWorkerTimings : public CHeapObj<mtGC> {
   void print() const;
 };
 
-class ShenandoahTerminationTimings : public CHeapObj<mtGC> {
-private:
-  WorkerDataArray<double>* _gc_termination_phase;
-public:
-  ShenandoahTerminationTimings(uint max_gc_threads);
-
-  // record the time a phase took in seconds
-  void record_time_secs(uint worker_i, double secs);
-
-  double average() const;
-  void reset();
-
-  void print() const;
-};
-
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHPHASETIMINGS_HPP
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.cpp b/src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.cpp
@@ -31,8 +31,6 @@
 #include "runtime/os.hpp"
 
 
-ShenandoahPhaseTimings::Phase ShenandoahTerminationTracker::_current_termination_phase = ShenandoahPhaseTimings::_num_phases;
-
 ShenandoahWorkerTimingsTracker::ShenandoahWorkerTimingsTracker(ShenandoahWorkerTimings* worker_times,
                                                               ShenandoahPhaseTimings::GCParPhases phase, uint worker_id) :
   _phase(phase), _worker_times(worker_times), _worker_id(worker_id) {
@@ -53,44 +51,3 @@ ShenandoahWorkerTimingsTracker::~ShenandoahWorkerTimingsTracker() {
   }
 }
 
-ShenandoahTerminationTimingsTracker::ShenandoahTerminationTimingsTracker(uint worker_id) :
-  _worker_id(worker_id)  {
-  if (ShenandoahTerminationTrace) {
-    _start_time = os::elapsedTime();
-  }
-}
-
-ShenandoahTerminationTimingsTracker::~ShenandoahTerminationTimingsTracker() {
-  if (ShenandoahTerminationTrace) {
-    ShenandoahHeap::heap()->phase_timings()->termination_times()->record_time_secs(_worker_id, os::elapsedTime() - _start_time);
-  }
-}
-
-ShenandoahTerminationTracker::ShenandoahTerminationTracker(ShenandoahPhaseTimings::Phase phase) : _phase(phase) {
-  assert(_current_termination_phase == ShenandoahPhaseTimings::_num_phases, "Should be invalid");
-  assert(phase == ShenandoahPhaseTimings::termination ||
-         phase == ShenandoahPhaseTimings::final_traversal_gc_termination ||
-         phase == ShenandoahPhaseTimings::full_gc_mark_termination ||
-         phase == ShenandoahPhaseTimings::conc_termination ||
-         phase == ShenandoahPhaseTimings::conc_traversal_termination ||
-         phase == ShenandoahPhaseTimings::weakrefs_termination ||
-         phase == ShenandoahPhaseTimings::full_gc_weakrefs_termination,
-         "Only these phases");
-
-  assert(!Thread::current()->is_Worker_thread() &&
-             (Thread::current()->is_VM_thread() ||
-              Thread::current()->is_ConcurrentGC_thread()),
-        "Called from wrong thread");
-
-  _current_termination_phase = phase;
-  ShenandoahHeap::heap()->phase_timings()->termination_times()->reset();
-}
-
-ShenandoahTerminationTracker::~ShenandoahTerminationTracker() {
-  assert(_phase == _current_termination_phase, "Can not change phase");
-  ShenandoahPhaseTimings* phase_times = ShenandoahHeap::heap()->phase_timings();
-
-  double t = phase_times->termination_times()->average();
-  phase_times->record_phase_time(_phase, t);
-  debug_only(_current_termination_phase = ShenandoahPhaseTimings::_num_phases;)
-}
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.hpp b/src/hotspot/share/gc/shenandoah/shenandoahTimingTracker.hpp
@@ -42,28 +42,4 @@ class ShenandoahWorkerTimingsTracker : public StackObj {
     ~ShenandoahWorkerTimingsTracker();
 };
 
-
-class ShenandoahTerminationTimingsTracker : public StackObj {
-private:
-  double _start_time;
-  uint   _worker_id;
-
-public:
-  ShenandoahTerminationTimingsTracker(uint worker_id);
-  ~ShenandoahTerminationTimingsTracker();
-};
-
-// Tracking termination time in specific GC phase
-class ShenandoahTerminationTracker : public StackObj {
-private:
-  ShenandoahPhaseTimings::Phase _phase;
-
-  static ShenandoahPhaseTimings::Phase _current_termination_phase;
-public:
-  ShenandoahTerminationTracker(ShenandoahPhaseTimings::Phase phase);
-  ~ShenandoahTerminationTracker();
-
-  static ShenandoahPhaseTimings::Phase current_termination_phase() { return _current_termination_phase; }
-};
-
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHTIMINGTRACKER_HPP
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp
@@ -538,7 +538,6 @@ void ShenandoahTraversalGC::main_loop_work(T* cl, jushort* live_data, uint worke
     if (work == 0) {
       // No more work, try to terminate
       ShenandoahSuspendibleThreadSetLeaver stsl(sts_yield && ShenandoahSuspendibleWorkers);
-      ShenandoahTerminationTimingsTracker term_tracker(worker_id);
       ShenandoahTerminatorTerminator tt(_heap);
 
       if (terminator->offer_termination(&tt)) return;
@@ -558,7 +557,6 @@ void ShenandoahTraversalGC::concurrent_traversal_collection() {
   if (!_heap->cancelled_gc()) {
     uint nworkers = _heap->workers()->active_workers();
     task_queues()->reserve(nworkers);
-    ShenandoahTerminationTracker tracker(ShenandoahPhaseTimings::conc_traversal_termination);
 
     TaskTerminator terminator(nworkers, task_queues());
     ShenandoahConcurrentTraversalCollectionTask task(&terminator);
@@ -581,8 +579,6 @@ void ShenandoahTraversalGC::final_traversal_collection() {
 
     // Finish traversal
     ShenandoahAllRootScanner rp(nworkers, ShenandoahPhaseTimings::final_traversal_gc_work);
-    ShenandoahTerminationTracker term(ShenandoahPhaseTimings::final_traversal_gc_termination);
-
     TaskTerminator terminator(nworkers, task_queues());
     ShenandoahFinalTraversalCollectionTask task(&rp, &terminator);
     _heap->workers()->run_task(&task);
diff --git a/src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp b/src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp
@@ -298,9 +298,6 @@
   diagnostic(bool, ShenandoahAllocFailureALot, false,                       \
           "Make lots of artificial allocation failures.")                   \
                                                                             \
-  diagnostic(bool, ShenandoahTerminationTrace, false,                       \
-          "Tracing task termination timings")                               \
-                                                                            \
   diagnostic(bool, ShenandoahAlwaysPreTouch, false,                         \
           "Pre-touch heap memory, overrides global AlwaysPreTouch")         \
                                                                             \

Original file line number	Diff line number	Diff line change
`@@ -298,9 +298,6 @@`
`298`	`298`	`diagnostic(bool, ShenandoahAllocFailureALot, false, \`
`299`	`299`	`"Make lots of artificial allocation failures.") \`
`300`	`300`	`\`
`301`		`- diagnostic(bool, ShenandoahTerminationTrace, false, \`
`302`		`- "Tracing task termination timings") \`
`303`		`- \`
`304`	`301`	`diagnostic(bool, ShenandoahAlwaysPreTouch, false, \`
`305`	`302`	`"Pre-touch heap memory, overrides global AlwaysPreTouch") \`
`306`	`303`	`\`