RFR: 8248132: ZGC: Unify handling of all OopStorage instances in root processing

[Per Liden]

Hi Stefan,

On 6/23/20 10:10 AM, Stefan Karlsson wrote:
> Hi all,
> 
> Please review this patch to unify handling of all OopStorage instances 
> in root processing.
> 
> https://cr.openjdk.java.net/~stefank/8248132/webrev.01/

I note that the size of the dynamic allocation is always known at 
compile-time. So how about we just avoid it altogether with something 
like this?

diff --git a/src/hotspot/share/gc/shared/oopStorageSetParState.hpp 
b/src/hotspot/share/gc/shared/oopStorageSetParState.hpp
--- a/src/hotspot/share/gc/shared/oopStorageSetParState.hpp
+++ b/src/hotspot/share/gc/shared/oopStorageSetParState.hpp
@@ -26,16 +26,16 @@
  #define SHARE_GC_SHARED_OOPSTORAGESETPARSTATE_HPP

  #include "gc/shared/oopStorageParState.hpp"
+#include "gc/shared/oopStorageSet.hpp"

  template <bool concurrent, bool is_const>
  class OopStorageSetStrongParState {
  private:
    typedef OopStorage::ParState<concurrent, is_const> ParStateType;

-  ParStateType* _par_states;
+  char _par_states[sizeof(ParStateType) * OopStorageSet::strong_count];

-  static ParStateType* allocate();
-  static void deallocate(ParStateType* iter_set);
+  ParStateType* par_state(int index);

  public:
    OopStorageSetStrongParState();
diff --git 
a/src/hotspot/share/gc/shared/oopStorageSetParState.inline.hpp 
b/src/hotspot/share/gc/shared/oopStorageSetParState.inline.hpp
--- a/src/hotspot/share/gc/shared/oopStorageSetParState.inline.hpp
+++ b/src/hotspot/share/gc/shared/oopStorageSetParState.inline.hpp
@@ -26,25 +26,19 @@
  #define SHARE_GC_SHARED_OOPSTORAGESETPARSTATE_INLINE_HPP

  #include "gc/shared/oopStorageParState.inline.hpp"
-#include "gc/shared/oopStorageSet.hpp"
  #include "gc/shared/oopStorageSetParState.hpp"

  template <bool concurrent, bool is_const>
-typename OopStorageSetStrongParState<concurrent, 
is_const>::ParStateType* OopStorageSetStrongParState<concurrent, 
is_const>::allocate() {
-  return 
MallocArrayAllocator<ParStateType>::allocate(OopStorageSet::strong_count, 
mtGC);
-}
-
-template <bool concurrent, bool is_const>
-void OopStorageSetStrongParState<concurrent, 
is_const>::deallocate(ParStateType* iter_set) {
-  MallocArrayAllocator<ParStateType>::free(iter_set);
+typename OopStorageSetStrongParState<concurrent, 
is_const>::ParStateType* OopStorageSetStrongParState<concurrent, 
is_const>::par_state(int index) {
+  return reinterpret_cast<ParStateType*>(_par_states) + index;
  }

  template <bool concurrent, bool is_const>
  OopStorageSetStrongParState<concurrent, 
is_const>::OopStorageSetStrongParState() :
-    _par_states(allocate()) {
+    _par_states() {
    int counter = 0;
    for (OopStorageSet::Iterator it = OopStorageSet::strong_iterator(); 
!it.is_end(); ++it) {
-    new (&_par_states[counter++]) ParStateType(*it);
+    new (par_state(counter++)) ParStateType(*it);
    }
  }

@@ -52,17 +46,15 @@
  OopStorageSetStrongParState<concurrent, 
is_const>::~OopStorageSetStrongParState() {
    int counter = 0;
    for (OopStorageSet::Iterator it = OopStorageSet::strong_iterator(); 
!it.is_end(); ++it) {
-    _par_states[counter++].~ParStateType();
+    par_state(counter++)->~ParStateType();
    }
-
-  deallocate(_par_states);
  }

  template <bool concurrent, bool is_const>
  template <typename Closure>
  void OopStorageSetStrongParState<concurrent, 
is_const>::oops_do(Closure* cl) {
    for (size_t i = 0; i < OopStorageSet::strong_count; i++) {
-    _par_states[i].oops_do(cl);
+    par_state(i)->oops_do(cl);
    }
  }


/Per

> https://bugs.openjdk.java.net/browse/JDK-8248132
> 
> This removes the explicit enumeration of "strong" OopStorages in ZGC. 
> This is a step towards allowing the Runtime code to add new OopStorages 
> without having to update all GCs.
> 
> Tested with tier1-3
> 
> Thanks,
> StefanK