8257676: Simplify WeakProcessorPhase

Reviewed-by: iwalulya, ayang, tschatzl

src/hotspot/share/gc/shared/weakProcessor.cpp

@@ -29,7 +29,6 @@
 #include "gc/shared/oopStorageSet.hpp"
 #include "gc/shared/weakProcessor.inline.hpp"
 #include "gc/shared/oopStorageSetParState.inline.hpp"
-#include "gc/shared/weakProcessorPhases.hpp"
 #include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "memory/allocation.inline.hpp"
 #include "memory/iterator.hpp"

src/hotspot/share/gc/shared/weakProcessor.inline.hpp

@@ -29,7 +29,7 @@
 #include "gc/shared/oopStorage.inline.hpp"
 #include "gc/shared/oopStorageParState.inline.hpp"
 #include "gc/shared/weakProcessor.hpp"
-#include "gc/shared/weakProcessorPhases.hpp"
+#include "gc/shared/weakProcessorPhase.hpp"
 #include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "gc/shared/workgroup.hpp"
 #include "prims/resolvedMethodTable.hpp"
@@ -81,13 +81,12 @@ void WeakProcessor::Task::work(uint worker_id,
          "worker_id (%u) exceeds task's configured workers (%u)",
          worker_id, _nworkers);
 
-  typedef WeakProcessorPhases::Iterator Iterator;
-
-  for (Iterator it = WeakProcessorPhases::oopstorage_iterator(); !it.is_end(); ++it) {
-    WeakProcessorPhase phase = *it;
+  constexpr EnumRange<WeakProcessorPhase> phase_range{};
+  for (WeakProcessorPhase phase : phase_range) {
     CountingClosure<IsAlive, KeepAlive> cl(is_alive, keep_alive);
     WeakProcessorPhaseTimeTracker pt(_phase_times, phase, worker_id);
-    StorageState* cur_state = _storage_states.par_state(phase);
+    int state_index = checked_cast<int>(phase_range.index(phase));
+    StorageState* cur_state = _storage_states.par_state(state_index);
     cur_state->oops_do(&cl);
     cur_state->increment_num_dead(cl.dead());
     if (_phase_times != NULL) {

src/hotspot/share/gc/shared/weakProcessorPhase.hpp

@@ -22,25 +22,14 @@
  *
  */
 
-#include "precompiled.hpp"
-#include "gc/shared/weakProcessorPhases.hpp"
-#include "utilities/debug.hpp"
-#include "utilities/macros.hpp"
+#ifndef SHARE_GC_SHARED_WEAKPROCESSORPHASE_HPP
+#define SHARE_GC_SHARED_WEAKPROCESSORPHASE_HPP
 
-#ifdef ASSERT
+#include "gc/shared/oopStorageSet.hpp"
+#include "utilities/enumIterator.hpp"
 
-void WeakProcessorPhases::Iterator::verify_nonsingular() const {
-  assert(_limit != singular_value, "precondition");
-}
+enum class WeakProcessorPhase : uint {};
 
-void WeakProcessorPhases::Iterator::verify_category_match(const Iterator& other) const {
-  verify_nonsingular();
-  assert(_limit == other._limit, "precondition");
-}
+ENUMERATOR_VALUE_RANGE(WeakProcessorPhase, 0, OopStorageSet::weak_count);
 
-void WeakProcessorPhases::Iterator::verify_dereferenceable() const {
-  verify_nonsingular();
-  assert(_index < _limit, "precondition");
-}
-
-#endif // ASSERT
+#endif // SHARE_GC_SHARED_WEAKPROCESSORPHASE_HPP

src/hotspot/share/gc/shared/weakProcessorPhaseTimes.cpp

@@ -24,7 +24,7 @@
 
 #include "precompiled.hpp"
 #include "gc/shared/oopStorage.hpp"
-#include "gc/shared/weakProcessorPhases.hpp"
+#include "gc/shared/weakProcessorPhase.hpp"
 #include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "gc/shared/workerDataArray.inline.hpp"
 #include "logging/log.hpp"
@@ -100,7 +100,9 @@ void WeakProcessorPhaseTimes::record_total_time_sec(double time_sec) {
 }
 
 WorkerDataArray<double>* WeakProcessorPhaseTimes::worker_data(WeakProcessorPhase phase) const {
-  return _worker_data[phase];
+  size_t index = EnumRange<WeakProcessorPhase>().index(phase);
+  assert(index < ARRAY_SIZE(_worker_data), "invalid phase");
+  return _worker_data[index];
 }
 
 double WeakProcessorPhaseTimes::worker_time_sec(uint worker_id, WeakProcessorPhase phase) const {
@@ -203,9 +205,8 @@ void WeakProcessorPhaseTimes::log_phase_details(WorkerDataArray<T>* data,
 
 void WeakProcessorPhaseTimes::log_print_phases(uint indent) const {
   if (log_is_enabled(Debug, gc, phases)) {
-    typedef WeakProcessorPhases::Iterator Iterator;
-    for (Iterator it = WeakProcessorPhases::oopstorage_iterator(); !it.is_end(); ++it) {
-      log_phase_summary(*it, indent);
+    for (WeakProcessorPhase phase : EnumRange<WeakProcessorPhase>()) {
+      log_phase_summary(phase, indent);
     }
   }
 }

src/hotspot/share/gc/shared/weakProcessorPhaseTimes.hpp

@@ -25,7 +25,7 @@
 #ifndef SHARE_GC_SHARED_WEAKPROCESSORPHASETIMES_HPP
 #define SHARE_GC_SHARED_WEAKPROCESSORPHASETIMES_HPP
 
-#include "gc/shared/weakProcessorPhases.hpp"
+#include "gc/shared/weakProcessorPhase.hpp"
 #include "memory/allocation.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/ticks.hpp"
@@ -44,9 +44,7 @@ class WeakProcessorPhaseTimes {
   double _total_time_sec;
 
   // Per-worker times and linked items.
-  static const uint worker_data_count = WeakProcessorPhases::oopstorage_phase_count;
-  WorkerDataArray<double>* _worker_data[worker_data_count];
-
+  WorkerDataArray<double>* _worker_data[EnumRange<WeakProcessorPhase>().size()];
   WorkerDataArray<double>* worker_data(WeakProcessorPhase phase) const;
 
   void log_phase_summary(WeakProcessorPhase phase, uint indent) const;

src/hotspot/share/gc/shared/weakProcessorPhases.hpp

@@ -1,118 +0,0 @@
-/*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- *
- */
-
-#ifndef SHARE_GC_SHARED_WEAKPROCESSORPHASES_HPP
-#define SHARE_GC_SHARED_WEAKPROCESSORPHASES_HPP
-
-#include "gc/shared/oopStorageSet.hpp"
-#include "memory/allocation.hpp"
-#include "utilities/globalDefinitions.hpp"
-#include "utilities/macros.hpp"
-
-class BoolObjectClosure;
-class OopClosure;
-class OopStorage;
-
-class WeakProcessorPhases : AllStatic {
-public:
-  class Iterator;
-
-  enum Phase {
-    // Implicit phase values for oopstorages.
-  };
-
-  static const uint oopstorage_phase_start = 0;
-  static const uint oopstorage_phase_count = OopStorageSet::weak_count;
-  static const uint phase_count = oopstorage_phase_count;
-
-  static Iterator oopstorage_iterator();
-};
-
-typedef WeakProcessorPhases::Phase WeakProcessorPhase;
-
-class WeakProcessorPhases::Iterator {
-  friend class WeakProcessorPhases;
-
-  uint _index;
-  uint _limit;
-
-  Iterator(uint index, uint limit) : _index(index), _limit(limit) {}
-
-  static const uint singular_value = UINT_MAX;
-  void verify_nonsingular() const NOT_DEBUG_RETURN;
-  void verify_category_match(const Iterator& other) const NOT_DEBUG_RETURN;
-  void verify_dereferenceable() const NOT_DEBUG_RETURN;
-
-public:
-  // Construct a singular iterator for later assignment.  The only valid
-  // operations are destruction and assignment.
-  Iterator() : _index(singular_value), _limit(singular_value) {}
-
-  bool is_end() const {
-    verify_nonsingular();
-    return _index == _limit;
-  }
-
-  bool operator==(const Iterator& other) const {
-    verify_category_match(other);
-    return _index == other._index;
-  }
-
-  bool operator!=(const Iterator& other) const {
-    return !operator==(other);
-  }
-
-  WeakProcessorPhase operator*() const {
-    verify_dereferenceable();
-    return static_cast<WeakProcessorPhase>(_index);
-  }
-
-  // Phase doesn't have members, so no operator->().
-  Iterator& operator++() {
-    verify_dereferenceable();
-    ++_index;
-    return *this;
-  }
-
-  Iterator operator++(int) {
-    verify_dereferenceable();
-    return Iterator(_index++, _limit);
-  }
-
-  Iterator begin() const {
-    verify_nonsingular();
-    return *this;
-  }
-
-  Iterator end() const {
-    verify_nonsingular();
-    return Iterator(_limit, _limit);
-  }
-};
-
-inline WeakProcessorPhases::Iterator WeakProcessorPhases::oopstorage_iterator() {
-  return Iterator(oopstorage_phase_start, oopstorage_phase_start + oopstorage_phase_count);
-}
-
-#endif // SHARE_GC_SHARED_WEAKPROCESSORPHASES_HPP

src/hotspot/share/utilities/enumIterator.hpp

@@ -40,7 +40,6 @@
 //
 // case 2:
 // enum has sequential values, with U start and U end (exclusive).
-// WeakProcessorPhases is an example because of oopstorage.
 // This can be mapped onto case 1 by casting start/(end-1).
 //
 // case 3: