Address feedback on code style and uninitialized check

neethu-prasad · Aug 6, 2024 · a7c0514 · a7c0514
1 parent 6e7fdd5
commit a7c0514
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 16 deletions.
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp b/src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.cpp
@@ -137,21 +137,21 @@ bool ShenandoahPhaseTimings::is_root_work_phase(Phase phase) {
   }
 }
 
-void ShenandoahPhaseTimings::set_cycle_data(Phase phase, double time, bool should_aggregate_cycles) {
-  if (should_aggregate_cycles) {
-    _cycle_data[phase] = _cycle_data[phase] <= 0 ? time :  _cycle_data[phase] + 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;
   }
 }
 
-void ShenandoahPhaseTimings::record_phase_time(Phase phase, double time, bool should_aggregate_cycles) {
+void ShenandoahPhaseTimings::record_phase_time(Phase phase, double time, bool should_aggregate) {
   if (!_policy->is_at_shutdown()) {
-    set_cycle_data(phase, time, should_aggregate_cycles);
+    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
@@ -217,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, bool should_aggregate_cycles=false);
+  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, bool should_aggregate_cycles=false);
+  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,20 +111,18 @@ ShenandoahConcurrentPhase::~ShenandoahConcurrentPhase() {
   _timer->register_gc_concurrent_end();
 }
 
-ShenandoahTimingsTracker::ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase, bool should_aggregate_cycles) :
+ShenandoahTimingsTracker::ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase, bool should_aggregate) :
   _timings(ShenandoahHeap::heap()->phase_timings()), _phase(phase) {
   assert(Thread::current()->is_VM_thread() || Thread::current()->is_ConcurrentGC_thread(),
           "Must be set by these threads");
   _parent_phase = _current_phase;
   _current_phase = phase;
   _start = os::elapsedTime();
-  _should_aggregate_cycles = should_aggregate_cycles;
+  _should_aggregate = should_aggregate;
 }
 
 ShenandoahTimingsTracker::~ShenandoahTimingsTracker() {
-  const double end_time = os::elapsedTime();
-  const double phase_elapsed_time = end_time - _start;
-  _timings->record_phase_time(_phase, phase_elapsed_time, _should_aggregate_cycles);
+  _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
@@ -66,10 +66,10 @@ class ShenandoahTimingsTracker : public StackObj {
   const ShenandoahPhaseTimings::Phase   _phase;
   ShenandoahPhaseTimings::Phase         _parent_phase;
   double _start;
-  bool _should_aggregate_cycles;
+  bool _should_aggregate;
 
 public:
-  ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase, bool should_aggregate_cycles=false);
+  ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase, bool should_aggregate = false);
   ~ShenandoahTimingsTracker();
 
   static ShenandoahPhaseTimings::Phase current_phase() { return _current_phase; }