8306123: Move InstanceKlass writeable flags

coleenp · coleenp · commit ddb86469e024 · 2023-04-19T14:06:02.000Z
Reviewed-by: iklam, fparain
diff --git a/src/hotspot/share/oops/instanceKlass.hpp b/src/hotspot/share/oops/instanceKlass.hpp
@@ -223,15 +223,11 @@ class InstanceKlass: public Klass {
 
   volatile u2     _idnum_allocated_count;   // JNI/JVMTI: increments with the addition of methods, old ids don't change
 
-  // _is_marked_dependent can be set concurrently, thus cannot be part of the
-  // _misc_flags right now.
-  bool            _is_marked_dependent;     // used for marking during flushing and deoptimization
-
   volatile ClassState _init_state;          // state of class
 
   u1              _reference_type;          // reference type
 
-  // State is set while executing, eventually atomically to not disturb other state
+  // State is set either at parse time or while executing, atomically to not disturb other state
   InstanceKlassFlags _misc_flags;
 
   Monitor*             _init_monitor;       // mutual exclusion to _init_state and _init_thread.
@@ -531,8 +527,8 @@ class InstanceKlass: public Klass {
   void set_should_verify_class(bool value) { _misc_flags.set_should_verify_class(value); }
 
   // marking
-  bool is_marked_dependent() const         { return _is_marked_dependent; }
-  void set_is_marked_dependent(bool value) { _is_marked_dependent = value; }
+  bool is_marked_dependent() const         { return _misc_flags.is_marked_dependent(); }
+  void set_is_marked_dependent(bool value) { _misc_flags.set_is_marked_dependent(value); }
 
   // initialization (virtuals from Klass)
   bool should_be_initialized() const;  // means that initialize should be called
@@ -681,16 +677,8 @@ class InstanceKlass: public Klass {
   // Redefinition locking.  Class can only be redefined by one thread at a time.
   // The flag is in access_flags so that it can be set and reset using atomic
   // operations, and not be reset by other misc_flag settings.
-  bool is_being_redefined() const          {
-    return _access_flags.is_being_redefined();
-  }
-  void set_is_being_redefined(bool value)  {
-    if (value) {
-      _access_flags.set_is_being_redefined();
-    } else {
-      _access_flags.clear_is_being_redefined();
-    }
-  }
+  bool is_being_redefined() const          { return _misc_flags.is_being_redefined(); }
+  void set_is_being_redefined(bool value)  { _misc_flags.set_is_being_redefined(value); }
 
   // RedefineClasses() support for previous versions:
   void add_previous_version(InstanceKlass* ik, int emcp_method_count);
@@ -716,13 +704,8 @@ class InstanceKlass: public Klass {
   bool is_scratch_class() const { return _misc_flags.is_scratch_class(); }
   void set_is_scratch_class() { _misc_flags.set_is_scratch_class(true); }
 
-  bool has_resolved_methods() const {
-    return _access_flags.has_resolved_methods();
-  }
-
-  void set_has_resolved_methods() {
-    _access_flags.set_has_resolved_methods();
-  }
+  bool has_resolved_methods() const { return _misc_flags.has_resolved_methods(); }
+  void set_has_resolved_methods()   { _misc_flags.set_has_resolved_methods(true); }
 
 public:
 #if INCLUDE_JVMTI
diff --git a/src/hotspot/share/oops/instanceKlassFlags.cpp b/src/hotspot/share/oops/instanceKlassFlags.cpp
@@ -26,9 +26,31 @@
 #include "classfile/classLoader.hpp"
 #include "classfile/classLoaderData.inline.hpp"
 #include "oops/instanceKlassFlags.hpp"
+#include "runtime/atomic.hpp"
 #include "runtime/safepoint.hpp"
 #include "utilities/macros.hpp"
 
+// This can be removed for the atomic bitset functions, when available.
+void InstanceKlassFlags::atomic_set_bits(u1 bits) {
+  // Atomically update the status with the bits given
+  u1 old_status, new_status, f;
+  do {
+    old_status = _status;
+    new_status = old_status | bits;
+    f = Atomic::cmpxchg(&_status, old_status, new_status);
+  } while(f != old_status);
+}
+
+void InstanceKlassFlags::atomic_clear_bits(u1 bits) {
+  // Atomically update the status with the bits given
+  u1 old_status, new_status, f;
+  do {
+    old_status = _status;
+    new_status = old_status & ~bits;
+    f = Atomic::cmpxchg(&_status, old_status, new_status);
+  } while(f != old_status);
+}
+
 #if INCLUDE_CDS
 void InstanceKlassFlags::set_shared_class_loader_type(s2 loader_type) {
   switch (loader_type) {
diff --git a/src/hotspot/share/oops/instanceKlassFlags.hpp b/src/hotspot/share/oops/instanceKlassFlags.hpp
@@ -27,6 +27,12 @@
 
 class ClassLoaderData;
 
+// The InstanceKlassFlags class contains the parse-time and writeable flags associated with
+// an InstanceKlass, and their associated accessors.
+// _flags are parse-time and constant in the InstanceKlass after that.  _status are set at runtime and
+// require atomic access.
+// These flags are JVM internal and not part of the AccessFlags classfile specification.
+
 class InstanceKlassFlags {
   friend class VMStructs;
   friend class JVMCIVMStructs;
@@ -35,32 +41,45 @@ class InstanceKlassFlags {
     flag(rewritten                          , 1 << 0) /* methods rewritten. */ \
     flag(has_nonstatic_fields               , 1 << 1) /* for sizing with UseCompressedOops */ \
     flag(should_verify_class                , 1 << 2) /* allow caching of preverification */ \
-    flag(unused                             , 1 << 3) /* not currently used */ \
-    flag(is_contended                       , 1 << 4) /* marked with contended annotation */ \
-    flag(has_nonstatic_concrete_methods     , 1 << 5) /* class/superclass/implemented interfaces has non-static, concrete methods */ \
-    flag(declares_nonstatic_concrete_methods, 1 << 6) /* directly declares non-static, concrete methods */ \
-    flag(has_been_redefined                 , 1 << 7) /* class has been redefined */ \
-    flag(shared_loading_failed              , 1 << 8) /* class has been loaded from shared archive */ \
-    flag(is_scratch_class                   , 1 << 9) /* class is the redefined scratch class */ \
-    flag(is_shared_boot_class               , 1 << 10) /* defining class loader is boot class loader */ \
-    flag(is_shared_platform_class           , 1 << 11) /* defining class loader is platform class loader */ \
-    flag(is_shared_app_class                , 1 << 12) /* defining class loader is app class loader */ \
-    flag(has_contended_annotations          , 1 << 13) /* has @Contended annotation */ \
-    flag(has_localvariable_table            , 1 << 14) /* has localvariable information */
+    flag(is_contended                       , 1 << 3) /* marked with contended annotation */ \
+    flag(has_nonstatic_concrete_methods     , 1 << 4) /* class/superclass/implemented interfaces has non-static, concrete methods */ \
+    flag(declares_nonstatic_concrete_methods, 1 << 5) /* directly declares non-static, concrete methods */ \
+    flag(shared_loading_failed              , 1 << 6) /* class has been loaded from shared archive */ \
+    flag(is_shared_boot_class               , 1 << 7) /* defining class loader is boot class loader */ \
+    flag(is_shared_platform_class           , 1 << 8) /* defining class loader is platform class loader */ \
+    flag(is_shared_app_class                , 1 << 9) /* defining class loader is app class loader */ \
+    flag(has_contended_annotations          , 1 << 10) /* has @Contended annotation */ \
+    flag(has_localvariable_table            , 1 << 11) /* has localvariable information */
 
 #define IK_FLAGS_ENUM_NAME(name, value)    _misc_##name = value,
   enum {
     IK_FLAGS_DO(IK_FLAGS_ENUM_NAME)
   };
 #undef IK_FLAGS_ENUM_NAME
 
+#define IK_STATUS_DO(status)  \
+    status(is_being_redefined                , 1 << 0) /* True if the klass is being redefined */ \
+    status(has_resolved_methods              , 1 << 1) /* True if the klass has resolved MethodHandle methods */ \
+    status(has_been_redefined                , 1 << 2) /* class has been redefined */ \
+    status(is_scratch_class                  , 1 << 3) /* class is the redefined scratch class */ \
+    status(is_marked_dependent               , 1 << 4) /* class is the redefined scratch class */
+
+#define IK_STATUS_ENUM_NAME(name, value)    _misc_##name = value,
+  enum {
+    IK_STATUS_DO(IK_STATUS_ENUM_NAME)
+  };
+#undef IK_STATUS_ENUM_NAME
+
   u2 shared_loader_type_bits() const {
     return _misc_is_shared_boot_class|_misc_is_shared_platform_class|_misc_is_shared_app_class;
   }
 
   // These flags are write-once before the class is published and then read-only so don't require atomic updates.
   u2 _flags;
 
+  // These flags are written during execution so require atomic stores
+  u1 _status;
+
  public:
 
   InstanceKlassFlags() : _flags(0) {}
@@ -87,6 +106,26 @@ class InstanceKlassFlags {
 
   void assign_class_loader_type(const ClassLoaderData* cld);
   void assert_is_safe(bool set) NOT_DEBUG_RETURN;
+
+  // Create getters and setters for the status values.
+#define IK_STATUS_GET(name, ignore)          \
+  bool name() const { return (_status & _misc_##name) != 0; }
+  IK_STATUS_DO(IK_STATUS_GET)
+#undef IK_STATUS_GET
+
+#define IK_STATUS_SET(name, ignore)   \
+  void set_##name(bool b) {         \
+    if (b) { \
+      atomic_set_bits(_misc_##name); \
+    } else { \
+      atomic_clear_bits(_misc_##name); \
+    } \
+  }
+  IK_STATUS_DO(IK_STATUS_SET)
+#undef IK_STATUS_SET
+
+  void atomic_set_bits(u1 bits);
+  void atomic_clear_bits(u1 bits);
 };
 
 #endif // SHARE_OOPS_INSTANCEKLASSFLAGS_HPP
diff --git a/src/hotspot/share/runtime/vmStructs.cpp b/src/hotspot/share/runtime/vmStructs.cpp
@@ -236,7 +236,6 @@
   nonstatic_field(InstanceKlass,               _static_field_size,                            int)                                   \
   nonstatic_field(InstanceKlass,               _static_oop_field_count,                       u2)                                    \
   nonstatic_field(InstanceKlass,               _nonstatic_oop_map_size,                       int)                                   \
-  nonstatic_field(InstanceKlass,               _is_marked_dependent,                          bool)                                  \
   volatile_nonstatic_field(InstanceKlass,      _init_state,                                   InstanceKlass::ClassState)             \
   volatile_nonstatic_field(InstanceKlass,      _init_thread,                                  JavaThread*)                           \
   nonstatic_field(InstanceKlass,               _itable_len,                                   int)                                   \
diff --git a/src/hotspot/share/utilities/accessFlags.hpp b/src/hotspot/share/utilities/accessFlags.hpp
@@ -68,8 +68,6 @@ enum {
   JVM_ACC_HAS_FINAL_METHOD        = 0x01000000,     // True if klass has final method
   JVM_ACC_IS_HIDDEN_CLASS         = 0x04000000,     // True if klass is hidden
   JVM_ACC_IS_VALUE_BASED_CLASS    = 0x08000000,     // True if klass is marked as a ValueBased class
-  JVM_ACC_IS_BEING_REDEFINED      = 0x00100000,     // True if the klass is being redefined.
-  JVM_ACC_HAS_RESOLVED_METHODS    = 0x00200000,     // True if the klass has resolved methods
 
   // Method* flags
   JVM_ACC_HAS_LOCAL_VARIABLE_TABLE= 0x00400000,
@@ -133,13 +131,6 @@ class AccessFlags {
   void set_has_localvariable_table()    { atomic_set_bits(JVM_ACC_HAS_LOCAL_VARIABLE_TABLE); }
   void clear_has_localvariable_table()  { atomic_clear_bits(JVM_ACC_HAS_LOCAL_VARIABLE_TABLE); }
 
-  bool is_being_redefined() const       { return (_flags & JVM_ACC_IS_BEING_REDEFINED) != 0; }
-  void set_is_being_redefined()         { atomic_set_bits(JVM_ACC_IS_BEING_REDEFINED); }
-  void clear_is_being_redefined()       { atomic_clear_bits(JVM_ACC_IS_BEING_REDEFINED); }
-
-  bool has_resolved_methods() const     { return (_flags & JVM_ACC_HAS_RESOLVED_METHODS) != 0; }
-  void set_has_resolved_methods()       { atomic_set_bits(JVM_ACC_HAS_RESOLVED_METHODS); }
-
   bool on_stack() const                 { return (_flags & JVM_ACC_ON_STACK) != 0; }
 
   // get .class file flags
diff --git a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java
@@ -80,7 +80,6 @@ private static synchronized void initialize(TypeDataBase db) throws WrongTypeExc
     staticFieldSize      = new CIntField(type.getCIntegerField("_static_field_size"), 0);
     staticOopFieldCount  = new CIntField(type.getCIntegerField("_static_oop_field_count"), 0);
     nonstaticOopMapSize  = new CIntField(type.getCIntegerField("_nonstatic_oop_map_size"), 0);
-    isMarkedDependent    = new CIntField(type.getCIntegerField("_is_marked_dependent"), 0);
     initState            = new CIntField(type.getCIntegerField("_init_state"), 0);
     itableLen            = new CIntField(type.getCIntegerField("_itable_len"), 0);
     if (VM.getVM().isJvmtiSupported()) {
@@ -145,7 +144,6 @@ public InstanceKlass(Address addr) {
   private static CIntField staticFieldSize;
   private static CIntField staticOopFieldCount;
   private static CIntField nonstaticOopMapSize;
-  private static CIntField isMarkedDependent;
   private static CIntField initState;
   private static CIntField itableLen;
   private static AddressField breakpoints;
@@ -373,7 +371,6 @@ public int getAllFieldsCount() {
   public long      getNonstaticFieldSize()  { return                nonstaticFieldSize.getValue(this); }
   public long      getStaticOopFieldCount() { return                staticOopFieldCount.getValue(this); }
   public long      getNonstaticOopMapSize() { return                nonstaticOopMapSize.getValue(this); }
-  public boolean   getIsMarkedDependent()   { return                isMarkedDependent.getValue(this) != 0; }
   public long      getItableLen()           { return                itableLen.getValue(this); }
   public long      majorVersion()           { return                getConstants().majorVersion(); }
   public long      minorVersion()           { return                getConstants().minorVersion(); }
@@ -571,7 +568,6 @@ public void iterateFields(MetadataVisitor visitor) {
       visitor.doCInt(staticFieldSize, true);
       visitor.doCInt(staticOopFieldCount, true);
       visitor.doCInt(nonstaticOopMapSize, true);
-      visitor.doCInt(isMarkedDependent, true);
       visitor.doCInt(initState, true);
       visitor.doCInt(itableLen, true);
     }
