8256814: WeakProcessorPhases may be redundant

Remove WeakProcessorPhase, adding scoped enum categories to OopStorageSet.

Reviewed-by: stefank, tschatzl, rkennke

src/hotspot/share/gc/g1/g1FullGCAdjustTask.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, 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
@@ -31,7 +31,6 @@
 #include "gc/g1/g1StringDedup.hpp"
 #include "gc/g1/heapRegionManager.hpp"
 #include "gc/shared/parallelCleaning.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "gc/shared/weakProcessor.hpp"
 #include "utilities/ticks.hpp"
 

src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp

@@ -31,6 +31,7 @@
 #include "gc/g1/g1StringDedup.hpp"
 #include "gc/shared/gcTimer.hpp"
 #include "gc/shared/oopStorage.hpp"
+#include "gc/shared/oopStorageSet.hpp"
 #include "gc/shared/tlab_globals.hpp"
 #include "gc/shared/workerDataArray.inline.hpp"
 #include "memory/resourceArea.hpp"
@@ -38,6 +39,7 @@
 #include "logging/logStream.hpp"
 #include "runtime/timer.hpp"
 #include "runtime/os.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/macros.hpp"
 
 static const char* indent(uint level) {
@@ -64,13 +66,14 @@ G1GCPhaseTimes::G1GCPhaseTimes(STWGCTimer* gc_timer, uint max_gc_threads) :
   AOT_ONLY(_gc_par_phases[AOTCodeRoots] = new WorkerDataArray<double>("AOTCodeRoots", "AOT Root Scan (ms):", max_gc_threads);)
   _gc_par_phases[CMRefRoots] = new WorkerDataArray<double>("CMRefRoots", "CM RefProcessor Roots (ms):", max_gc_threads);
 
-  int index = G1GCPhaseTimes::StrongOopStorageSetRoots;
+  for (auto id : EnumRange<OopStorageSet::StrongId>()) {
-  for (OopStorageSet::Iterator it = OopStorageSet::strong_iterator(); !it.is_end(); ++it, ++index) {
+    GCParPhases phase = strong_oopstorage_phase(id);
     const char* phase_name_postfix = " Roots (ms):";
-    char* oop_storage_phase_name = NEW_C_HEAP_ARRAY(char, strlen(phase_name_postfix) + strlen(it->name()) + 1, mtGC);
+    const char* storage_name = OopStorageSet::storage(id)->name();
-    strcpy(oop_storage_phase_name, it->name());
+    char* oop_storage_phase_name = NEW_C_HEAP_ARRAY(char, strlen(phase_name_postfix) + strlen(storage_name) + 1, mtGC);
+    strcpy(oop_storage_phase_name, storage_name);
     strcat(oop_storage_phase_name, phase_name_postfix);
-    _gc_par_phases[index] = new WorkerDataArray<double>(it->name(), oop_storage_phase_name, max_gc_threads);
+    _gc_par_phases[phase] = new WorkerDataArray<double>(storage_name, oop_storage_phase_name, max_gc_threads);
   }
 
   _gc_par_phases[MergeER] = new WorkerDataArray<double>("MergeER", "Eager Reclaim (ms):", max_gc_threads);
@@ -487,7 +490,8 @@ double G1GCPhaseTimes::print_post_evacuate_collection_set() const {
 
   debug_time_for_reference("Reference Processing", _cur_ref_proc_time_ms);
   _ref_phase_times.print_all_references(2, false);
-  _weak_phase_times.log_print(2);
+  _weak_phase_times.log_total(2);
+  _weak_phase_times.log_subtotals(3);
 
   if (G1StringDedup::is_enabled()) {
     debug_time("String Deduplication", _cur_string_deduplication_time_ms);

src/hotspot/share/gc/g1/g1GCPhaseTimes.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2021, 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
@@ -27,9 +27,10 @@
 
 #include "gc/shared/oopStorageSet.hpp"
 #include "gc/shared/referenceProcessorPhaseTimes.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
+#include "gc/shared/weakProcessorTimes.hpp"
 #include "logging/logLevel.hpp"
 #include "memory/allocation.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/macros.hpp"
 
 class LineBuffer;
@@ -51,10 +52,10 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
     CLDGRoots,
     AOT_ONLY(AOTCodeRoots COMMA)
     CMRefRoots,
-    // For every OopStorage there will be one element in the enum, starting with
+    // For every strong OopStorage there will be one element in this enum,
-    // StrongOopStorageSetRoots.
+    // starting with StrongOopStorageSetRoots.
     StrongOopStorageSetRoots,
-    MergeER = StrongOopStorageSetRoots + OopStorageSet::strong_count,
+    MergeER = StrongOopStorageSetRoots + EnumRange<OopStorageSet::StrongId>().size(),
     MergeRS,
     OptMergeRS,
     MergeLB,
@@ -84,6 +85,11 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
   static const GCParPhases ExtRootScanSubPhasesFirst = ThreadRoots;
   static const GCParPhases ExtRootScanSubPhasesLast = GCParPhases(MergeER - 1);
 
+  static constexpr GCParPhases strong_oopstorage_phase(OopStorageSet::StrongId id) {
+    size_t index = EnumRange<OopStorageSet::StrongId>().index(id);
+    return GCParPhases(StrongOopStorageSetRoots + index);
+  }
+
   enum GCMergeRSWorkTimes {
     MergeRSMergedSparse,
     MergeRSMergedFine,
@@ -187,7 +193,7 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
   double _cur_verify_after_time_ms;
 
   ReferenceProcessorPhaseTimes _ref_phase_times;
-  WeakProcessorPhaseTimes _weak_phase_times;
+  WeakProcessorTimes _weak_phase_times;
 
   double worker_time(GCParPhases phase, uint worker);
   void note_gc_end();
@@ -436,7 +442,7 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
 
   ReferenceProcessorPhaseTimes* ref_phase_times() { return &_ref_phase_times; }
 
-  WeakProcessorPhaseTimes* weak_phase_times() { return &_weak_phase_times; }
+  WeakProcessorTimes* weak_phase_times() { return &_weak_phase_times; }
 };
 
 class G1EvacPhaseWithTrimTimeTracker : public StackObj {

src/hotspot/share/gc/g1/g1RootProcessor.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2021, 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
@@ -44,6 +44,7 @@
 #include "gc/shared/referenceProcessor.hpp"
 #include "memory/allocation.inline.hpp"
 #include "runtime/mutex.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/macros.hpp"
 
 G1RootProcessor::G1RootProcessor(G1CollectedHeap* g1h, uint n_workers) :
@@ -195,10 +196,10 @@ void G1RootProcessor::process_vm_roots(G1RootClosures* closures,
   }
 #endif
 
-  for (int i = 0; i < _oop_storage_set_strong_par_state.par_state_count(); ++i) {
+  for (auto id : EnumRange<OopStorageSet::StrongId>()) {
-    G1GCPhaseTimes::GCParPhases phase = G1GCPhaseTimes::GCParPhases(G1GCPhaseTimes::StrongOopStorageSetRoots + i);
+    G1GCPhaseTimes::GCParPhases phase = G1GCPhaseTimes::strong_oopstorage_phase(id);
     G1GCParPhaseTimesTracker x(phase_times, phase, worker_id);
-    _oop_storage_set_strong_par_state.par_state(i)->oops_do(closures->strong_oops());
+    _oop_storage_set_strong_par_state.par_state(id)->oops_do(closures->strong_oops());
   }
 }
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, 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
@@ -29,14 +29,13 @@
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"
 
-// +1 for NULL singular entry.
+OopStorage* OopStorageSet::_storages[all_count] = {};
-OopStorage* OopStorageSet::storages[all_count + 1] = {};
 
 OopStorage* OopStorageSet::create_strong(const char* name) {
   static uint registered_strong = 0;
   assert(registered_strong < strong_count, "More registered strong storages than slots");
   OopStorage* storage = new OopStorage(name);
-  storages[strong_start + registered_strong++] = storage;
+  _storages[strong_start + registered_strong++] = storage;
   return storage;
 }
 
@@ -44,47 +43,50 @@ OopStorage* OopStorageSet::create_weak(const char* name) {
   static uint registered_weak = 0;
   assert(registered_weak < weak_count, "More registered strong storages than slots");
   OopStorage* storage = new OopStorage(name);
-  storages[weak_start + registered_weak++] = storage;
+  _storages[weak_start + registered_weak++] = storage;
   return storage;
 }
 
 
 void OopStorageSet::fill_strong(OopStorage* to[strong_count]) {
   for (uint i = 0; i < OopStorageSet::strong_count; i++) {
-    to[i] = storage(strong_start + i);
+    to[i] = get_storage(strong_start + i);
   }
 }
 
 void OopStorageSet::fill_weak(OopStorage* to[weak_count]) {
   for (uint i = 0; i < OopStorageSet::weak_count; i++) {
-    to[i] = storage(weak_start + i);
+    to[i] = get_storage(weak_start + i);
   }
 }
 
 void OopStorageSet::fill_all(OopStorage* to[all_count]) {
   for (uint i = 0; i < OopStorageSet::all_count; i++) {
-    to[i] = storage(all_start + i);
+    to[i] = get_storage(all_start + i);
   }
 }
 
+OopStorage* OopStorageSet::get_storage(uint index) {
+  verify_initialized(index);
+  return _storages[index];
+}
+
+template<typename E>
+OopStorage* OopStorageSet::get_storage(E id) {
+  assert(EnumRange<E>().first() <= id, "invalid id");
+  assert(id <= EnumRange<E>().last(), "invalid id");
+  return get_storage(static_cast<uint>(id));
+}
+
+template OopStorage* OopStorageSet::get_storage(StrongId);
+template OopStorage* OopStorageSet::get_storage(WeakId);
+template OopStorage* OopStorageSet::get_storage(Id);
+
 #ifdef ASSERT
 
 void OopStorageSet::verify_initialized(uint index) {
-  assert(storages[index] != NULL, "oopstorage_init not yet called");
+  assert(index < ARRAY_SIZE(_storages), "invalid index");
-}
+  assert(_storages[index] != NULL, "oopstorage_init not yet called");
-
-void OopStorageSet::Iterator::verify_nonsingular() const {
-  assert(_category != singular, "precondition");
-}
-
-void OopStorageSet::Iterator::verify_category_match(const Iterator& other) const {
-  verify_nonsingular();
-  assert(_category == other._category, "precondition");
-}
-
-void OopStorageSet::Iterator::verify_dereferenceable() const {
-  verify_nonsingular();
-  assert(!is_end(), "precondition");
 }
 
 #endif // ASSERT

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

@@ -27,6 +27,7 @@
 
 #include "memory/allocation.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"
 
@@ -35,29 +36,29 @@ class OopStorage;
 class OopStorageSet : public AllStatic {
   friend class OopStorageSetTest;
 
-public:
   // Must be updated when new OopStorages are introduced
   static const uint strong_count = 4 JVMTI_ONLY(+ 1);
   static const uint weak_count = 5 JVMTI_ONLY(+ 1) JFR_ONLY(+ 1);
-  static const uint all_count = strong_count + weak_count;
 
-private:
+  static const uint all_count = strong_count + weak_count;
-  static const uint singular_index = 0; // For singular iterator.
+  static const uint all_start = 0;
-  static const uint all_start = 1;
+  static const uint all_end = all_start + all_count;
+
   static const uint strong_start = all_start;
   static const uint strong_end = strong_start + strong_count;
+
   static const uint weak_start = strong_end;
   static const uint weak_end = weak_start + weak_count;
-  static const uint all_end = weak_end;
+  static_assert(all_end == weak_end, "invariant");
 
-  static OopStorage* storages[all_end];
+  static OopStorage* _storages[all_count];
 
   static void verify_initialized(uint index) NOT_DEBUG_RETURN;
 
-  static OopStorage* storage(uint index) {
+  static OopStorage* get_storage(uint index);
-    verify_initialized(index);
+
-    return storages[index];
+  template<typename E>
-  }
+  static OopStorage* get_storage(E id);
 
   // Testing support
   static void fill_strong(OopStorage* storage[strong_count]);
@@ -65,101 +66,68 @@ private:
   static void fill_all(OopStorage* storage[all_count]);
 
 public:
-  class Iterator;
+  enum class StrongId : uint {}; // [strong_start, strong_end)
+  enum class WeakId : uint {};   // [weak_start, weak_end)
+  enum class Id : uint {};       // [all_start, all_end)
 
-  static Iterator strong_iterator();
+  // Give these access to the private start/end/count constants.
-  static Iterator weak_iterator();
+  friend struct EnumeratorRange<StrongId>;
-  static Iterator all_iterator();
+  friend struct EnumeratorRange<WeakId>;
+  friend struct EnumeratorRange<Id>;
+
+  static OopStorage* storage(StrongId id) { return get_storage(id); }
+  static OopStorage* storage(WeakId id) { return get_storage(id); }
+  static OopStorage* storage(Id id) { return get_storage(id); }
 
   static OopStorage* create_strong(const char* name);
   static OopStorage* create_weak(const char* name);
 
+  // Support iteration over the storage objects.
+  template<typename StorageId> class Range;
+  template<typename StorageId> class Iterator;
+
   template <typename Closure>
   static void strong_oops_do(Closure* cl);
+
 };
 
+ENUMERATOR_VALUE_RANGE(OopStorageSet::StrongId,
+                       OopStorageSet::strong_start,
+                       OopStorageSet::strong_end);
+
+ENUMERATOR_VALUE_RANGE(OopStorageSet::WeakId,
+                       OopStorageSet::weak_start,
+                       OopStorageSet::weak_end);
+
+ENUMERATOR_VALUE_RANGE(OopStorageSet::Id,
+                       OopStorageSet::all_start,
+                       OopStorageSet::all_end);
+
+template<typename StorageId>
 class OopStorageSet::Iterator {
-  friend class OopStorageSet;
+  EnumIterator<StorageId> _it;
-
-  enum Category { singular, strong, weak, all };
-
-  uint _index;
-  uint _limit;
-  DEBUG_ONLY(Category _category;)
-
-  Iterator(uint index, uint limit, Category category) :
-    _index(index), _limit(limit) DEBUG_ONLY(COMMA _category(category)) {}
-
-  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
+  constexpr Iterator() : _it() {}
-  // operations are destruction and assignment.
+  explicit constexpr Iterator(EnumIterator<StorageId> it) : _it(it) {}
-  Iterator() :
-    _index(singular_index),
-    _limit(singular_index)
-    DEBUG_ONLY(COMMA _category(singular)) {}
 
-  bool is_end() const {
+  constexpr bool operator==(Iterator other) const { return _it == other._it; }
-    verify_nonsingular();
+  constexpr bool operator!=(Iterator other) const { return _it != other._it; }
-    return _index == _limit;
-  }
 
-  bool operator==(const Iterator& other) const {
+  constexpr Iterator& operator++() { ++_it; return *this; }
-    verify_category_match(other);
+  constexpr Iterator operator++(int) { Iterator i = *this; ++_it; return i; }
-    return _index == other._index;
-  }
 
-  bool operator!=(const Iterator& other) const {
+  OopStorage* operator*() const { return storage(*_it); }
-    return !operator==(other);
+  OopStorage* operator->() const { return operator*(); }
-  }
-
-  OopStorage* operator*() const {
-    verify_dereferenceable();
-    return storage(_index);
-  }
-
-  OopStorage* operator->() const {
-    return operator*();
-  }
-
-  Iterator& operator++() {
-    verify_dereferenceable();
-    ++_index;
-    return *this;
-  }
-
-  Iterator operator++(int) {
-    Iterator result = *this;
-    operator++();
-    return result;
-  }
-
-  Iterator begin() const {
-    verify_nonsingular();
-    return *this;
-  }
-
-  Iterator end() const {
-    verify_nonsingular();
-    Iterator result = *this;
-    result._index = _limit;
-    return result;
-  }
 };
 
-inline OopStorageSet::Iterator OopStorageSet::strong_iterator() {
+template<typename StorageId>
-  return Iterator(strong_start, strong_end, Iterator::strong);
+class OopStorageSet::Range {
-}
+  EnumRange<StorageId> _range;
 
-inline OopStorageSet::Iterator OopStorageSet::weak_iterator() {
+public:
-  return Iterator(weak_start, weak_end, Iterator::weak);
+  constexpr auto begin() const { return Iterator<StorageId>(_range.begin()); }
-}
+  constexpr auto end() const { return Iterator<StorageId>(_range.end()); }
-
+};
-inline OopStorageSet::Iterator OopStorageSet::all_iterator() {
-  return Iterator(all_start, all_end, Iterator::all);
-}
 
 #endif // SHARE_GC_SHARED_OOPSTORAGESET_HPP

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, 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
@@ -27,11 +27,12 @@
 
 #include "gc/shared/oopStorage.inline.hpp"
 #include "gc/shared/oopStorageSet.hpp"
+#include "utilities/enumIterator.hpp"
 
 template <typename Closure>
 void OopStorageSet::strong_oops_do(Closure* cl) {
-  for (OopStorageSet::Iterator it = OopStorageSet::strong_iterator(); !it.is_end(); ++it) {
+  for (auto id : EnumRange<StrongId>()) {
-    (*it)->oops_do(cl);
+    storage(id)->oops_do(cl);
   }
 }
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, 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
@@ -27,40 +27,48 @@
 
 #include "gc/shared/oopStorageParState.hpp"
 #include "gc/shared/oopStorageSet.hpp"
+#include "utilities/enumIterator.hpp"
+#include "utilities/globalDefinitions.hpp"
 #include "utilities/valueObjArray.hpp"
 
-template <bool concurrent, bool is_const>
+// Base class for OopStorageSet{Strong,Weak}ParState.
-class OopStorageSetStrongParState {
+template<typename StorageId, bool concurrent, bool is_const>
-  typedef OopStorage::ParState<concurrent, is_const> ParStateType;
+class OopStorageSetParState {
-  typedef ValueObjArray<ParStateType, OopStorageSet::strong_count> ParStateArray;
-
-  ParStateArray _par_states;
-
 public:
-  OopStorageSetStrongParState();
+  using ParState = OopStorage::ParState<concurrent, is_const>;
 
-  template <typename Closure>
+  ParState* par_state(StorageId id) const {
-  void oops_do(Closure* cl);
+    return _par_states.at(checked_cast<int>(EnumRange<StorageId>().index(id)));
+  }
 
-  ParStateType* par_state(int i) const { return _par_states.at(i); }
+protected:
-  int par_state_count() const { return _par_states.count(); }
+  OopStorageSetParState() : _par_states(OopStorageSet::Range<StorageId>().begin()) {}
+  ~OopStorageSetParState() = default;
+
+private:
+  ValueObjArray<ParState, EnumRange<StorageId>().size()> _par_states;
+
+  NONCOPYABLE(OopStorageSetParState);
 };
 
-template <bool concurrent, bool is_const>
+// Set of strong parallel states.
-class OopStorageSetWeakParState {
+template<bool concurrent, bool is_const>
-  typedef OopStorage::ParState<concurrent, is_const> ParStateType;
+class OopStorageSetStrongParState
-  typedef ValueObjArray<ParStateType, OopStorageSet::weak_count> ParStateArray;
+  : public OopStorageSetParState<OopStorageSet::StrongId, concurrent, is_const>
-
+{
-  ParStateArray _par_states;
-
 public:
-  OopStorageSetWeakParState();
+  template<typename Closure>
+  void oops_do(Closure* cl);
+};
 
-  template <typename ClosureType>
+// Set of weak parallel states.
-  void oops_do(ClosureType* cl);
+template<bool concurrent, bool is_const>
-
+class OopStorageSetWeakParState
-  ParStateType* par_state(int i) const { return _par_states.at(i); }
+  : public OopStorageSetParState<OopStorageSet::WeakId, concurrent, is_const>
-  int par_state_count() const { return _par_states.count(); }
+{
+public:
+  template<typename Closure>
+  void oops_do(Closure* cl);
 
   void report_num_dead();
 };

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, 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
@@ -33,23 +33,13 @@
 #include "utilities/debug.hpp"
 
 template <bool concurrent, bool is_const>
-OopStorageSetStrongParState<concurrent, is_const>::OopStorageSetStrongParState() :
+template <typename Closure>
-    _par_states(OopStorageSet::strong_iterator()) {
+void OopStorageSetStrongParState<concurrent, is_const>::oops_do(Closure* cl) {
-}
+  for (auto id : EnumRange<OopStorageSet::StrongId>()) {
-
+    this->par_state(id)->oops_do(cl);
-template <bool concurrent, bool is_const>
-template <typename ClosureType>
-void OopStorageSetStrongParState<concurrent, is_const>::oops_do(ClosureType* cl) {
-  for (int i = 0; i < _par_states.count(); i++) {
-    _par_states.at(i)->oops_do(cl);
   }
 }
 
-template <bool concurrent, bool is_const>
-OopStorageSetWeakParState<concurrent, is_const>::OopStorageSetWeakParState() :
-    _par_states(OopStorageSet::weak_iterator()) {
-}
-
 template <typename ClosureType>
 class DeadCounterClosure : public OopClosure {
 private:
@@ -78,12 +68,12 @@ public:
 };
 
 template <bool concurrent, bool is_const>
-template <typename ClosureType>
+template <typename Closure>
-void OopStorageSetWeakParState<concurrent, is_const>::oops_do(ClosureType* cl) {
+void OopStorageSetWeakParState<concurrent, is_const>::oops_do(Closure* cl) {
-  for (int i = 0; i < _par_states.count(); i++) {
+  for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-    ParStateType* state = _par_states.at(i);
+    auto state = this->par_state(id);
     if (state->storage()->should_report_num_dead()) {
-      DeadCounterClosure<ClosureType> counting_cl(cl);
+      DeadCounterClosure<Closure> counting_cl(cl);
       state->oops_do(&counting_cl);
       state->increment_num_dead(counting_cl.num_dead());
     } else {
@@ -94,9 +84,10 @@ void OopStorageSetWeakParState<concurrent, is_const>::oops_do(ClosureType* cl) {
 
 template <bool concurrent, bool is_const>
 void OopStorageSetWeakParState<concurrent, is_const>::report_num_dead() {
-  for (int i = 0; i < _par_states.count(); i++) {
+  for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-    ParStateType* state = _par_states.at(i);
+    auto state = this->par_state(id);
     state->storage()->report_num_dead(state->num_dead());
   }
 }
+
 #endif // SHARE_GC_SHARED_OOPSTORAGESETPARSTATE_INLINE_HPP

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

@@ -30,7 +30,7 @@
 #include "gc/shared/oopStorageSet.hpp"
 #include "gc/shared/weakProcessor.inline.hpp"
 #include "gc/shared/oopStorageSetParState.inline.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
+#include "gc/shared/weakProcessorTimes.hpp"
 #include "memory/allocation.inline.hpp"
 #include "memory/iterator.hpp"
 #include "prims/resolvedMethodTable.hpp"
@@ -62,23 +62,20 @@ void WeakProcessor::weak_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_a
 
   notify_jvmti_tagmaps();
 
-  OopStorageSet::Iterator it = OopStorageSet::weak_iterator();
+  for (OopStorage* storage : OopStorageSet::Range<OopStorageSet::WeakId>()) {
-  for ( ; !it.is_end(); ++it) {
+    if (storage->should_report_num_dead()) {
-    if (it->should_report_num_dead()) {
       CountingClosure<BoolObjectClosure, OopClosure> cl(is_alive, keep_alive);
-      it->oops_do(&cl);
+      storage->oops_do(&cl);
-      it->report_num_dead(cl.dead());
+      storage->report_num_dead(cl.dead());
     } else {
-      it->weak_oops_do(is_alive, keep_alive);
+      storage->weak_oops_do(is_alive, keep_alive);
     }
   }
 }
 
 void WeakProcessor::oops_do(OopClosure* closure) {
-
+  for (OopStorage* storage : OopStorageSet::Range<OopStorageSet::WeakId>()) {
-  OopStorageSet::Iterator it = OopStorageSet::weak_iterator();
+    storage->weak_oops_do(closure);
-  for ( ; !it.is_end(); ++it) {
-    it->weak_oops_do(closure);
   }
 }
 
@@ -92,9 +89,8 @@ uint WeakProcessor::ergo_workers(uint max_workers) {
   // One thread per ReferencesPerThread references (or fraction thereof)
   // in the various OopStorage objects, bounded by max_threads.
   size_t ref_count = 0;
-  OopStorageSet::Iterator it = OopStorageSet::weak_iterator();
+  for (OopStorage* storage : OopStorageSet::Range<OopStorageSet::WeakId>()) {
-  for ( ; !it.is_end(); ++it) {
+    ref_count += storage->allocation_count();
-    ref_count += it->allocation_count();
   }
 
   // +1 to (approx) round up the ref per thread division.
@@ -105,26 +101,20 @@ uint WeakProcessor::ergo_workers(uint max_workers) {
 
 void WeakProcessor::Task::initialize() {
   assert(_nworkers != 0, "must be");
-  assert(_phase_times == NULL || _nworkers <= _phase_times->max_threads(),
+  assert(_times == nullptr || _nworkers <= _times->max_threads(),
          "nworkers (%u) exceeds max threads (%u)",
-         _nworkers, _phase_times->max_threads());
+         _nworkers, _times->max_threads());
 
-  if (_phase_times) {
+  if (_times) {
-    _phase_times->set_active_workers(_nworkers);
+    _times->set_active_workers(_nworkers);
   }
   notify_jvmti_tagmaps();
 }
 
-WeakProcessor::Task::Task(uint nworkers) :
+WeakProcessor::Task::Task(uint nworkers) : Task(nullptr, nworkers) {}
-  _phase_times(NULL),
-  _nworkers(nworkers),
-  _storage_states()
-{
-  initialize();
-}
 
-WeakProcessor::Task::Task(WeakProcessorPhaseTimes* phase_times, uint nworkers) :
+WeakProcessor::Task::Task(WeakProcessorTimes* times, uint nworkers) :
-  _phase_times(phase_times),
+  _times(times),
   _nworkers(nworkers),
   _storage_states()
 {
@@ -132,10 +122,7 @@ WeakProcessor::Task::Task(WeakProcessorPhaseTimes* phase_times, uint nworkers) :
 }
 
 void WeakProcessor::Task::report_num_dead() {
-  for (int i = 0; i < _storage_states.par_state_count(); ++i) {
+  _storage_states.report_num_dead();
-    StorageState* state = _storage_states.par_state(i);
-    state->report_num_dead();
-  }
 }
 
 void WeakProcessor::GangTask::work(uint worker_id) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, 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
@@ -30,7 +30,7 @@
 #include "gc/shared/workgroup.hpp"
 #include "memory/allocation.hpp"
 
-class WeakProcessorPhaseTimes;
+class WeakProcessorTimes;
 class WorkGang;
 
 // Helper class to aid in root scanning and cleaning of weak oops in the VM.
@@ -56,7 +56,7 @@ public:
   static void weak_oops_do(WorkGang* workers,
                            IsAlive* is_alive,
                            KeepAlive* keep_alive,
-                           WeakProcessorPhaseTimes* phase_times);
+                           WeakProcessorTimes* times);
 
   // Convenience parallel version.  Uses ergo_workers() to determine the
   // number of threads to use, limited by the total workers.  Implicitly
@@ -85,7 +85,7 @@ private:
 class WeakProcessor::Task {
   typedef OopStorage::ParState<false, false> StorageState;
 
-  WeakProcessorPhaseTimes* _phase_times;
+  WeakProcessorTimes* _times;
   uint _nworkers;
   OopStorageSetWeakParState<false, false> _storage_states;
 
@@ -93,7 +93,7 @@ class WeakProcessor::Task {
 
 public:
   Task(uint nworkers);          // No time tracking.
-  Task(WeakProcessorPhaseTimes* phase_times, uint nworkers);
+  Task(WeakProcessorTimes* times, uint nworkers);
 
   template<typename IsAlive, typename KeepAlive>
   void work(uint worker_id, IsAlive* is_alive, KeepAlive* keep_alive);

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

@@ -28,12 +28,13 @@
 #include "classfile/stringTable.hpp"
 #include "gc/shared/oopStorage.inline.hpp"
 #include "gc/shared/oopStorageParState.inline.hpp"
+#include "gc/shared/oopStorageSet.hpp"
 #include "gc/shared/weakProcessor.hpp"
-#include "gc/shared/weakProcessorPhase.hpp"
+#include "gc/shared/weakProcessorTimes.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "gc/shared/workgroup.hpp"
 #include "prims/resolvedMethodTable.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/enumIterator.hpp"
 
 class BoolObjectClosure;
 class OopClosure;
@@ -81,16 +82,15 @@ void WeakProcessor::Task::work(uint worker_id,
          "worker_id (%u) exceeds task's configured workers (%u)",
          worker_id, _nworkers);
 
-  constexpr EnumRange<WeakProcessorPhase> phase_range{};
+  for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-  for (WeakProcessorPhase phase : phase_range) {
     CountingClosure<IsAlive, KeepAlive> cl(is_alive, keep_alive);
-    WeakProcessorPhaseTimeTracker pt(_phase_times, phase, worker_id);
+    WeakProcessorParTimeTracker pt(_times, id, worker_id);
-    int state_index = checked_cast<int>(phase_range.index(phase));
+    StorageState* cur_state = _storage_states.par_state(id);
-    StorageState* cur_state = _storage_states.par_state(state_index);
+    assert(cur_state->storage() == OopStorageSet::storage(id), "invariant");
     cur_state->oops_do(&cl);
     cur_state->increment_num_dead(cl.dead());
-    if (_phase_times != NULL) {
+    if (_times != NULL) {
-      _phase_times->record_worker_items(worker_id, phase, cl.new_dead(), cl.total());
+      _times->record_worker_items(worker_id, id, cl.new_dead(), cl.total());
     }
   }
 }
@@ -113,10 +113,10 @@ public:
   GangTask(const char* name,
            IsAlive* is_alive,
            KeepAlive* keep_alive,
-           WeakProcessorPhaseTimes* phase_times,
+           WeakProcessorTimes* times,
            uint nworkers) :
     AbstractGangTask(name),
-    _task(phase_times, nworkers),
+    _task(times, nworkers),
     _is_alive(is_alive),
     _keep_alive(keep_alive),
     _erased_do_work(&erased_do_work<IsAlive, KeepAlive>)
@@ -130,13 +130,13 @@ template<typename IsAlive, typename KeepAlive>
 void WeakProcessor::weak_oops_do(WorkGang* workers,
                                  IsAlive* is_alive,
                                  KeepAlive* keep_alive,
-                                 WeakProcessorPhaseTimes* phase_times) {
+                                 WeakProcessorTimes* times) {
-  WeakProcessorTimeTracker tt(phase_times);
+  WeakProcessorTimeTracker tt(times);
 
   uint nworkers = ergo_workers(MIN2(workers->total_workers(),
-                                    phase_times->max_threads()));
+                                    times->max_threads()));
 
-  GangTask task("Weak Processor", is_alive, keep_alive, phase_times, nworkers);
+  GangTask task("Weak Processor", is_alive, keep_alive, times, nworkers);
   workers->run_task(&task, nworkers);
   task.report_num_dead();
 }
@@ -147,9 +147,9 @@ void WeakProcessor::weak_oops_do(WorkGang* workers,
                                  KeepAlive* keep_alive,
                                  uint indent_log) {
   uint nworkers = ergo_workers(workers->total_workers());
-  WeakProcessorPhaseTimes pt(nworkers);
+  WeakProcessorTimes times(nworkers);
-  weak_oops_do(workers, is_alive, keep_alive, &pt);
+  weak_oops_do(workers, is_alive, keep_alive, &times);
-  pt.log_print_phases(indent_log);
+  times.log_subtotals(indent_log); // Caller logs total if desired.
 }
 
 #endif // SHARE_GC_SHARED_WEAKPROCESSOR_INLINE_HPP

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

@@ -1,35 +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_WEAKPROCESSORPHASE_HPP
-#define SHARE_GC_SHARED_WEAKPROCESSORPHASE_HPP
-
-#include "gc/shared/oopStorageSet.hpp"
-#include "utilities/enumIterator.hpp"
-
-enum class WeakProcessorPhase : uint {};
-
-ENUMERATOR_VALUE_RANGE(WeakProcessorPhase, 0, OopStorageSet::weak_count);
-
-#endif // SHARE_GC_SHARED_WEAKPROCESSORPHASE_HPP

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, 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
@@ -24,12 +24,13 @@
 
 #include "precompiled.hpp"
 #include "gc/shared/oopStorage.hpp"
-#include "gc/shared/weakProcessorPhase.hpp"
+#include "gc/shared/weakProcessor.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
+#include "gc/shared/weakProcessorTimes.hpp"
 #include "gc/shared/workerDataArray.inline.hpp"
 #include "logging/log.hpp"
 #include "logging/logStream.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/ticks.hpp"
 
@@ -40,7 +41,7 @@ static bool is_initialized_time(double t) { return t >= 0.0; }
 #endif // ASSERT
 
 
-WeakProcessorPhaseTimes::WeakProcessorPhaseTimes(uint max_threads) :
+WeakProcessorTimes::WeakProcessorTimes(uint max_threads) :
   _max_threads(max_threads),
   _active_workers(0),
   _total_time_sec(uninitialized_time),
@@ -49,10 +50,9 @@ WeakProcessorPhaseTimes::WeakProcessorPhaseTimes(uint max_threads) :
   assert(_max_threads > 0, "max_threads must not be zero");
 
   WorkerDataArray<double>** wpt = _worker_data;
-  OopStorageSet::Iterator it = OopStorageSet::weak_iterator();
+  for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-  for ( ; !it.is_end(); ++it) {
     assert(size_t(wpt - _worker_data) < ARRAY_SIZE(_worker_data), "invariant");
-    const char* description = it->name();
+    const char* description = OopStorageSet::storage(id)->name();
     *wpt = new WorkerDataArray<double>(NULL, description, _max_threads);
     (*wpt)->create_thread_work_items("Dead", DeadItems);
     (*wpt)->create_thread_work_items("Total", TotalItems);
@@ -61,27 +61,27 @@ WeakProcessorPhaseTimes::WeakProcessorPhaseTimes(uint max_threads) :
   assert(size_t(wpt - _worker_data) == ARRAY_SIZE(_worker_data), "invariant");
 }
 
-WeakProcessorPhaseTimes::~WeakProcessorPhaseTimes() {
+WeakProcessorTimes::~WeakProcessorTimes() {
   for (size_t i = 0; i < ARRAY_SIZE(_worker_data); ++i) {
     delete _worker_data[i];
   }
 }
 
-uint WeakProcessorPhaseTimes::max_threads() const { return _max_threads; }
+uint WeakProcessorTimes::max_threads() const { return _max_threads; }
 
-uint WeakProcessorPhaseTimes::active_workers() const {
+uint WeakProcessorTimes::active_workers() const {
   assert(_active_workers != 0, "active workers not set");
   return _active_workers;
 }
 
-void WeakProcessorPhaseTimes::set_active_workers(uint n) {
+void WeakProcessorTimes::set_active_workers(uint n) {
   assert(_active_workers == 0, "active workers already set");
   assert(n > 0, "active workers must be non-zero");
   assert(n <= _max_threads, "active workers must not exceed max threads");
   _active_workers = n;
 }
 
-void WeakProcessorPhaseTimes::reset() {
+void WeakProcessorTimes::reset() {
   _active_workers = 0;
   _total_time_sec = uninitialized_time;
   for (size_t i = 0; i < ARRAY_SIZE(_worker_data); ++i) {
@@ -89,48 +89,49 @@ void WeakProcessorPhaseTimes::reset() {
   }
 }
 
-double WeakProcessorPhaseTimes::total_time_sec() const {
+double WeakProcessorTimes::total_time_sec() const {
   assert(is_initialized_time(_total_time_sec), "Total time not set");
   return _total_time_sec;
 }
 
-void WeakProcessorPhaseTimes::record_total_time_sec(double time_sec) {
+void WeakProcessorTimes::record_total_time_sec(double time_sec) {
   assert(!is_initialized_time(_total_time_sec), "Already set total time");
   _total_time_sec = time_sec;
 }
 
-WorkerDataArray<double>* WeakProcessorPhaseTimes::worker_data(WeakProcessorPhase phase) const {
+WorkerDataArray<double>* WeakProcessorTimes::worker_data(OopStorageSet::WeakId id) const {
-  size_t index = EnumRange<WeakProcessorPhase>().index(phase);
+  size_t index = EnumRange<OopStorageSet::WeakId>().index(id);
   assert(index < ARRAY_SIZE(_worker_data), "invalid phase");
   return _worker_data[index];
 }
 
-double WeakProcessorPhaseTimes::worker_time_sec(uint worker_id, WeakProcessorPhase phase) const {
+double WeakProcessorTimes::worker_time_sec(uint worker_id,
+                                           OopStorageSet::WeakId id) const {
   assert(worker_id < active_workers(),
          "invalid worker id %u for %u", worker_id, active_workers());
-  return worker_data(phase)->get(worker_id);
+  return worker_data(id)->get(worker_id);
 }
 
-void WeakProcessorPhaseTimes::record_worker_time_sec(uint worker_id,
+void WeakProcessorTimes::record_worker_time_sec(uint worker_id,
-                                                     WeakProcessorPhase phase,
+                                                OopStorageSet::WeakId id,
                                                 double time_sec) {
-  worker_data(phase)->set(worker_id, time_sec);
+  worker_data(id)->set(worker_id, time_sec);
 }
 
-void WeakProcessorPhaseTimes::record_worker_items(uint worker_id,
+void WeakProcessorTimes::record_worker_items(uint worker_id,
-                                                  WeakProcessorPhase phase,
+                                             OopStorageSet::WeakId id,
                                              size_t num_dead,
                                              size_t num_total) {
-  WorkerDataArray<double>* phase_data = worker_data(phase);
+  WorkerDataArray<double>* data = worker_data(id);
-  phase_data->set_or_add_thread_work_item(worker_id, num_dead, DeadItems);
+  data->set_or_add_thread_work_item(worker_id, num_dead, DeadItems);
-  phase_data->set_or_add_thread_work_item(worker_id, num_total, TotalItems);
+  data->set_or_add_thread_work_item(worker_id, num_total, TotalItems);
 }
 
 static double elapsed_time_sec(Ticks start_time, Ticks end_time) {
   return (end_time - start_time).seconds();
 }
 
-WeakProcessorTimeTracker::WeakProcessorTimeTracker(WeakProcessorPhaseTimes* times) :
+WeakProcessorTimeTracker::WeakProcessorTimeTracker(WeakProcessorTimes* times) :
   _times(times),
   _start_time(Ticks::now())
 {}
@@ -142,11 +143,11 @@ WeakProcessorTimeTracker::~WeakProcessorTimeTracker() {
   }
 }
 
-WeakProcessorPhaseTimeTracker::WeakProcessorPhaseTimeTracker(WeakProcessorPhaseTimes* times,
+WeakProcessorParTimeTracker::WeakProcessorParTimeTracker(WeakProcessorTimes* times,
-                                                             WeakProcessorPhase phase,
+                                                         OopStorageSet::WeakId storage_id,
                                                          uint worker_id) :
   _times(times),
-  _phase(phase),
+  _storage_id(storage_id),
   _worker_id(worker_id),
   _start_time(Ticks::now())
 {
@@ -155,10 +156,10 @@ WeakProcessorPhaseTimeTracker::WeakProcessorPhaseTimeTracker(WeakProcessorPhaseT
 }
 
 
-WeakProcessorPhaseTimeTracker::~WeakProcessorPhaseTimeTracker() {
+WeakProcessorParTimeTracker::~WeakProcessorParTimeTracker() {
   if (_times != NULL) {
     double time_sec = elapsed_time_sec(_start_time, Ticks::now());
-    _times->record_worker_time_sec(_worker_id, _phase, time_sec);
+    _times->record_worker_time_sec(_worker_id, _storage_id, time_sec);
   }
 }
 
@@ -174,27 +175,25 @@ static const char* indent_str(size_t i) {
 
 #define TIME_FORMAT "%.1lfms"
 
-void WeakProcessorPhaseTimes::log_phase_summary(WeakProcessorPhase phase,
+void WeakProcessorTimes::log_summary(OopStorageSet::WeakId id, uint indent) const {
-                                                uint indent) const {
   LogTarget(Debug, gc, phases) lt;
   LogStream ls(lt);
   ls.print("%s", indents[indent]);
-  worker_data(phase)->print_summary_on(&ls, true);
+  worker_data(id)->print_summary_on(&ls, true);
-  log_phase_details(worker_data(phase), indent + 1);
+  log_details(worker_data(id), indent + 1);
 
-  for (uint i = 0; i < worker_data(phase)->MaxThreadWorkItems; i++) {
+  for (uint i = 0; i < worker_data(id)->MaxThreadWorkItems; i++) {
-    WorkerDataArray<size_t>* work_items = worker_data(phase)->thread_work_items(i);
+    WorkerDataArray<size_t>* work_items = worker_data(id)->thread_work_items(i);
     if (work_items != NULL) {
       ls.print("%s", indents[indent + 1]);
       work_items->print_summary_on(&ls, true);
-      log_phase_details(work_items, indent + 1);
+      log_details(work_items, indent + 1);
     }
   }
 }
 
 template <typename T>
-void WeakProcessorPhaseTimes::log_phase_details(WorkerDataArray<T>* data,
+void WeakProcessorTimes::log_details(WorkerDataArray<T>* data, uint indent) const {
-                                                uint indent) const {
   LogTarget(Trace, gc, phases) lt;
   if (lt.is_enabled()) {
     LogStream ls(lt);
@@ -203,20 +202,17 @@ void WeakProcessorPhaseTimes::log_phase_details(WorkerDataArray<T>* data,
   }
 }
 
-void WeakProcessorPhaseTimes::log_print_phases(uint indent) const {
+void WeakProcessorTimes::log_subtotals(uint indent) const {
   if (log_is_enabled(Debug, gc, phases)) {
-    for (WeakProcessorPhase phase : EnumRange<WeakProcessorPhase>()) {
+    for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-      log_phase_summary(phase, indent);
+      log_summary(id, indent);
     }
   }
 }
 
-void WeakProcessorPhaseTimes::log_print(uint indent) const {
+void WeakProcessorTimes::log_total(uint indent) const {
-  if (log_is_enabled(Debug, gc, phases)) {
   log_debug(gc, phases)("%s%s: " TIME_FORMAT,
                         indent_str(indent),
                         "Weak Processing",
                         total_time_sec() * MILLIUNITS);
-    log_print_phases(indent + 1);
-  }
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, 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
@@ -22,17 +22,17 @@
  *
  */
 
-#ifndef SHARE_GC_SHARED_WEAKPROCESSORPHASETIMES_HPP
+#ifndef SHARE_GC_SHARED_WEAKPROCESSORTIMES_HPP
-#define SHARE_GC_SHARED_WEAKPROCESSORPHASETIMES_HPP
+#define SHARE_GC_SHARED_WEAKPROCESSORTIMES_HPP
 
-#include "gc/shared/weakProcessorPhase.hpp"
+#include "gc/shared/oopStorageSet.hpp"
 #include "memory/allocation.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/ticks.hpp"
 
 template<typename T> class WorkerDataArray;
 
-class WeakProcessorPhaseTimes {
+class WeakProcessorTimes {
   enum {
     DeadItems,
     TotalItems
@@ -44,63 +44,67 @@ class WeakProcessorPhaseTimes {
   double _total_time_sec;
 
   // Per-worker times and linked items.
-  WorkerDataArray<double>* _worker_data[EnumRange<WeakProcessorPhase>().size()];
+  WorkerDataArray<double>* _worker_data[EnumRange<OopStorageSet::WeakId>().size()];
-  WorkerDataArray<double>* worker_data(WeakProcessorPhase phase) const;
+  WorkerDataArray<double>* worker_data(OopStorageSet::WeakId id) const;
 
-  void log_phase_summary(WeakProcessorPhase phase, uint indent) const;
+  void log_summary(OopStorageSet::WeakId id, uint indent) const;
   template <typename T>
-  void log_phase_details(WorkerDataArray<T>* data, uint indent) const;
+  void log_details(WorkerDataArray<T>* data, uint indent) const;
 
 public:
-  WeakProcessorPhaseTimes(uint max_threads);
+  WeakProcessorTimes(uint max_threads);
-  ~WeakProcessorPhaseTimes();
+  ~WeakProcessorTimes();
 
   uint max_threads() const;
   uint active_workers() const;
   void set_active_workers(uint n);
 
   double total_time_sec() const;
-  double worker_time_sec(uint worker_id, WeakProcessorPhase phase) const;
+  double worker_time_sec(uint worker_id, OopStorageSet::WeakId id) const;
 
   void record_total_time_sec(double time_sec);
-  void record_worker_time_sec(uint worker_id, WeakProcessorPhase phase, double time_sec);
+  void record_worker_time_sec(uint worker_id,
-  void record_worker_items(uint worker_id, WeakProcessorPhase phase, size_t num_dead, size_t num_total);
+                              OopStorageSet::WeakId id,
+                              double time_sec);
+  void record_worker_items(uint worker_id,
+                           OopStorageSet::WeakId id,
+                           size_t num_dead,
+                           size_t num_total);
 
   void reset();
 
-  void log_print(uint indent = 0) const;
+  void log_total(uint indent = 0) const;
-  void log_print_phases(uint indent = 0) const;
+  void log_subtotals(uint indent = 0) const;
 };
 
 // Record total weak processor time and worker count in times.
 // Does nothing if times is NULL.
 class WeakProcessorTimeTracker : StackObj {
-  WeakProcessorPhaseTimes* _times;
+  WeakProcessorTimes* _times;
   Ticks _start_time;
 
 public:
-  WeakProcessorTimeTracker(WeakProcessorPhaseTimes* times);
+  WeakProcessorTimeTracker(WeakProcessorTimes* times);
   ~WeakProcessorTimeTracker();
 };
 
-// Record phase time contribution for the current thread in phase times.
+// Record time contribution for the current thread.
-// Does nothing if phase times is NULL.
+// Does nothing if times is NULL.
-class WeakProcessorPhaseTimeTracker : StackObj {
+class WeakProcessorParTimeTracker : StackObj {
-private:
+  WeakProcessorTimes* _times;
-  WeakProcessorPhaseTimes* _times;
+  OopStorageSet::WeakId _storage_id;
-  WeakProcessorPhase _phase;
   uint _worker_id;
   Ticks _start_time;
 
 public:
-  // For tracking possibly parallel phase times (even if processed by
+  // For tracking possibly parallel times (even if processed by
   // only one thread).
   // Precondition: worker_id < times->max_threads().
-  WeakProcessorPhaseTimeTracker(WeakProcessorPhaseTimes* times,
+  WeakProcessorParTimeTracker(WeakProcessorTimes* times,
-                                WeakProcessorPhase phase,
+                              OopStorageSet::WeakId storage_id,
                               uint worker_id);
 
-  ~WeakProcessorPhaseTimeTracker();
+  ~WeakProcessorParTimeTracker();
 };
 
-#endif // SHARE_GC_SHARED_WEAKPROCESSORPHASETIMES_HPP
+#endif // SHARE_GC_SHARED_WEAKPROCESSORTIMES_HPP

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2019, 2021, Red Hat, Inc. 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
@@ -27,7 +27,6 @@
 
 #include "gc/shared/parallelCleaning.hpp"
 #include "gc/shared/weakProcessor.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "gc/shared/workgroup.hpp"
 #include "gc/shenandoah/shenandoahRootProcessor.inline.hpp"
 #include "memory/iterator.hpp"

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2019, 2021, Red Hat, Inc. 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
@@ -38,6 +38,7 @@
 #include "gc/shared/oopStorageSet.hpp"
 #include "runtime/thread.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/enumIterator.hpp"
 
 ShenandoahGCStateResetter::ShenandoahGCStateResetter() :
   _heap(ShenandoahHeap::heap()),
@@ -154,8 +155,7 @@ void ShenandoahRootVerifier::strong_roots_do(OopClosure* oops) {
 }
 
 void ShenandoahRootVerifier::weak_roots_do(OopClosure* cl) {
-  for (OopStorageSet::Iterator it = OopStorageSet::weak_iterator(); !it.is_end(); ++it) {
+  for (auto id : EnumRange<OopStorageSet::WeakId>()) {
-    OopStorage* storage = *it;
+    OopStorageSet::storage(id)->oops_do(cl);
-    storage->oops_do<OopClosure>(cl);
   }
 }

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

@@ -30,7 +30,6 @@
 #include "gc/shared/gcVMOperations.hpp"
 #include "gc/shared/isGCActiveMark.hpp"
 #include "gc/shared/suspendibleThreadSet.hpp"
-#include "gc/shared/weakProcessorPhaseTimes.hpp"
 #include "gc/shenandoah/shenandoahPhaseTimings.hpp"
 #include "gc/shenandoah/shenandoahThreadLocalData.hpp"
 #include "jfr/jfrEvents.hpp"

src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2021, 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
@@ -41,6 +41,7 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/vframe_hp.hpp"
 #include "services/management.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/growableArray.hpp"
 
 class ReferenceLocateClosure : public OopClosure {
@@ -130,14 +131,14 @@ bool ReferenceToRootClosure::do_cldg_roots() {
 }
 
 bool ReferenceToRootClosure::do_oop_storage_roots() {
-  int i = 0;
+  using Range = EnumRange<OopStorageSet::StrongId>;
-  for (OopStorageSet::Iterator it = OopStorageSet::strong_iterator(); !it.is_end(); ++it, ++i) {
+  for (auto id : Range()) {
     assert(!complete(), "invariant");
-    OopStorage* oop_storage = *it;
+    OopStorage* oop_storage = OopStorageSet::storage(id);
     OldObjectRoot::Type type = JNIHandles::is_global_storage(oop_storage) ?
                                OldObjectRoot::_global_jni_handle :
                                OldObjectRoot::_global_oop_handle;
-    OldObjectRoot::System system = OldObjectRoot::System(OldObjectRoot::_strong_oop_storage_set_first + i);
+    OldObjectRoot::System system = OldObjectRoot::System(OldObjectRoot::_strong_oop_storage_set_first + Range().index(id));
     ReferenceLocateClosure rlc(_callback, system, type, NULL);
     oop_storage->oops_do(&rlc);
     if (rlc.complete()) {

src/hotspot/share/jfr/leakprofiler/utilities/rootType.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, 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
@@ -27,17 +27,16 @@
 #include "gc/shared/oopStorageSet.hpp"
 #include "jfr/leakprofiler/utilities/rootType.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/enumIterator.hpp"
 
 OopStorage* OldObjectRoot::system_oop_storage(System system) {
   int val = int(system);
   if (val >= _strong_oop_storage_set_first && val <= _strong_oop_storage_set_last) {
-    int index = val - _strong_oop_storage_set_first;
+    using StrongId = OopStorageSet::StrongId;
-    int i = 0;
+    using Underlying = std::underlying_type_t<StrongId>;
-    for (OopStorageSet::Iterator it = OopStorageSet::strong_iterator(); !it.is_end(); ++it, ++i) {
+    auto first = static_cast<Underlying>(EnumRange<StrongId>().first());
-      if (i == index) {
+    auto id = static_cast<StrongId>(first + (val - _strong_oop_storage_set_first));
-        return *it;
+    return OopStorageSet::storage(id);
-      }
-    }
   }
   return NULL;
 }

src/hotspot/share/jfr/leakprofiler/utilities/rootType.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, 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
@@ -27,6 +27,7 @@
 
 #include "gc/shared/oopStorageSet.hpp"
 #include "memory/allocation.hpp"
+#include "utilities/enumIterator.hpp"
 
 class OldObjectRoot : public AllStatic {
  public:
@@ -35,7 +36,7 @@ class OldObjectRoot : public AllStatic {
     _universe,
     _threads,
     _strong_oop_storage_set_first,
-    _strong_oop_storage_set_last = _strong_oop_storage_set_first + OopStorageSet::strong_count - 1,
+    _strong_oop_storage_set_last = _strong_oop_storage_set_first + EnumRange<OopStorageSet::StrongId>().size() - 1,
     _class_loader_data,
     _code_cache,
     _aot,

src/hotspot/share/runtime/serviceThread.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, 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
@@ -126,9 +126,8 @@ void ServiceThread::initialize() {
 }
 
 static void cleanup_oopstorages() {
-  OopStorageSet::Iterator it = OopStorageSet::all_iterator();
+  for (OopStorage* storage : OopStorageSet::Range<OopStorageSet::Id>()) {
-  for ( ; !it.is_end(); ++it) {
+    storage->delete_empty_blocks();
-    it->delete_empty_blocks();
   }
 }
 

test/hotspot/gtest/gc/shared/test_oopStorageSet.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, 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
@@ -27,6 +27,7 @@
 #include "gc/shared/oopStorageSet.hpp"
 #include "memory/allocation.inline.hpp"
 #include "utilities/debug.hpp"
+#include "utilities/enumIterator.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"
 #include "unittest.hpp"
@@ -44,52 +45,54 @@ protected:
     return count;
   }
 
-  template <uint count>
+  template <uint count, typename Range>
-  static void check_iterator(OopStorageSet::Iterator it,
+  static void check_iteration(Range range, OopStorage* storages[count]) {
-      OopStorage* storages[count]) {
+    ASSERT_EQ(range.size(), count);
-    OopStorageSet::Iterator start = it;
+    for (auto id : range) {
-    ASSERT_EQ(start, it);
+      OopStorage* storage = OopStorageSet::storage(id);
-    for ( ; !it.is_end(); ++it) {
+      size_t index = find_storage<count>(storage, storages);
-      size_t index = find_storage<count>(*it, storages);
       ASSERT_LT(index, count);
-      storages[index] = NULL;
+      storages[index] = nullptr;
     }
-    ASSERT_NE(start, it);
-    const OopStorage* null_storage = NULL;
     for (uint i = 0; i < count; ++i) {
-      ASSERT_EQ(null_storage, storages[i]);
+      ASSERT_EQ(nullptr /* null_storage */, storages[i]);
     }
   }
 
-  template <uint count>
+  template<uint count, typename Range>
-  static void test_iterator(OopStorageSet::Iterator iterator,
+  static void test_iteration(Range range, void (*fill)(OopStorage*[count])) {
-      void (*fill)(OopStorage*[count])) {
     OopStorage* storages[count];
     fill(storages);
-    check_iterator<count>(iterator, storages);
+    check_iteration<count>(range, storages);
   }
 
-  static void test_strong_iterator() {
+  static void test_strong_iteration() {
-    test_iterator<OopStorageSet::strong_count>(
+    test_iteration<OopStorageSet::strong_count>(
-        OopStorageSet::strong_iterator(),
+      EnumRange<OopStorageSet::StrongId>(),
       &OopStorageSet::fill_strong);
 
   }
-  static void test_weak_iterator() {
+  static void test_weak_iteration() {
-    test_iterator<OopStorageSet::weak_count>(
+    test_iteration<OopStorageSet::weak_count>(
-        OopStorageSet::weak_iterator(),
+      EnumRange<OopStorageSet::WeakId>(),
       &OopStorageSet::fill_weak);
 
   }
-  static void test_all_iterator() {
+  static void test_all_iteration() {
-    test_iterator<OopStorageSet::all_count>(
+    test_iteration<OopStorageSet::all_count>(
-        OopStorageSet::all_iterator(),
+      EnumRange<OopStorageSet::Id>(),
       &OopStorageSet::fill_all);
   }
 };
 
-TEST_VM_F(OopStorageSetTest, iterators) {
+TEST_VM_F(OopStorageSetTest, strong_iteration) {
-  test_strong_iterator();
+  test_strong_iteration();
-  test_weak_iterator();
+}
-  test_all_iterator();
+
+TEST_VM_F(OopStorageSetTest, weak_iteration) {
+  test_weak_iteration();
+}
+
+TEST_VM_F(OopStorageSetTest, all_iteration) {
+  test_all_iteration();
 }