8336742: Shenandoah: Add more verbose logging/stats for mark terminat…

…ion attempts Reviewed-by: shade, wkemper, rkennke
sendaoYan · Aug 13, 2024 · 90527a5 · 90527a5
1 parent 8e682ac
commit 90527a5
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 13 deletions.
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp
@@ -206,7 +206,10 @@ void ShenandoahConcurrentMark::concurrent_mark() {
  }
 
  size_t before = qset.completed_buffers_num();
- Handshake::execute(&flush_satb);
+ {
+ ShenandoahTimingsTracker t(ShenandoahPhaseTimings::conc_mark_satb_flush, true);
+ Handshake::execute(&flush_satb);
+ }
  size_t after = qset.completed_buffers_num();
 
  if (before == after) {

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp
@@ -137,17 +137,21 @@ bool ShenandoahPhaseTimings::is_root_work_phase(Phase phase) {
  }
 }
 
-void ShenandoahPhaseTimings::set_cycle_data(Phase phase, double time) {
+void ShenandoahPhaseTimings::set_cycle_data(Phase phase, double time, bool should_aggregate) {
+ const double cycle_data = _cycle_data[phase];
+ if (should_aggregate) {
+ _cycle_data[phase] = (cycle_data == uninitialized()) ? time : (cycle_data + time);
+ } else {
 #ifdef ASSERT
- double d = _cycle_data[phase];
- assert(d == uninitialized(), "Should not be set yet: %s, current value: %lf", phase_name(phase), d);
+ assert(cycle_data == uninitialized(), "Should not be set yet: %s, current value: %lf", phase_name(phase), cycle_data);
 #endif
- _cycle_data[phase] = time;
+ _cycle_data[phase] = time;
+ }
 }
 
-void ShenandoahPhaseTimings::record_phase_time(Phase phase, double time) {
+void ShenandoahPhaseTimings::record_phase_time(Phase phase, double time, bool should_aggregate) {
  if (!_policy->is_at_shutdown()) {
- set_cycle_data(phase, time);
+ set_cycle_data(phase, time, should_aggregate);
  }
 }
 

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp
@@ -57,6 +57,7 @@ class outputStream;
  f(conc_mark_roots, "Concurrent Mark Roots ") \
  SHENANDOAH_PAR_PHASE_DO(conc_mark_roots, " CMR: ", f) \
  f(conc_mark, "Concurrent Marking") \
+ f(conc_mark_satb_flush, " Flush SATB") \
  \
  f(final_mark_gross, "Pause Final Mark (G)") \
  f(final_mark, "Pause Final Mark (N)") \
@@ -216,13 +217,13 @@ class ShenandoahPhaseTimings : public CHeapObj<mtGC> {
  ShenandoahWorkerData* worker_data(Phase phase, ParPhase par_phase);
  Phase worker_par_phase(Phase phase, ParPhase par_phase);
 
- void set_cycle_data(Phase phase, double time);
+ void set_cycle_data(Phase phase, double time, bool should_aggregate = false);
  static double uninitialized() { return -1; }
 
 public:
  ShenandoahPhaseTimings(uint max_workers);
 
- void record_phase_time(Phase phase, double time);
+ void record_phase_time(Phase phase, double time, bool should_aggregate = false);
 
  void record_workers_start(Phase phase);
  void record_workers_end(Phase phase);

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp b/src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp
@@ -111,8 +111,8 @@ ShenandoahConcurrentPhase::~ShenandoahConcurrentPhase() {
  _timer->register_gc_concurrent_end();
 }
 
-ShenandoahTimingsTracker::ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase) :
- _timings(ShenandoahHeap::heap()->phase_timings()), _phase(phase) {
+ShenandoahTimingsTracker::ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase, bool should_aggregate) :
+ _timings(ShenandoahHeap::heap()->phase_timings()), _phase(phase), _should_aggregate(should_aggregate) {
  assert(Thread::current()->is_VM_thread() || Thread::current()->is_ConcurrentGC_thread(),
  "Must be set by these threads");
  _parent_phase = _current_phase;
@@ -121,7 +121,7 @@ ShenandoahTimingsTracker::ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase
 }
 
 ShenandoahTimingsTracker::~ShenandoahTimingsTracker() {
- _timings->record_phase_time(_phase, os::elapsedTime() - _start);
+ _timings->record_phase_time(_phase, os::elapsedTime() - _start, _should_aggregate);
  _current_phase = _parent_phase;
 }
 

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp b/src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp
@@ -64,11 +64,12 @@ class ShenandoahTimingsTracker : public StackObj {
 
  ShenandoahPhaseTimings* const _timings;
  const ShenandoahPhaseTimings::Phase _phase;
+ const bool _should_aggregate;
  ShenandoahPhaseTimings::Phase _parent_phase;
  double _start;
 
 public:
- ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase);
+ ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase, bool should_aggregate = false);
  ~ShenandoahTimingsTracker();
 
  static ShenandoahPhaseTimings::Phase current_phase() { return _current_phase; }