8365823: Revert storing abstract and interface Klasses to non-class metaspace

coleenp · coleenp · commit fa00b24954d6 · 2025-09-19T11:54:34.000Z
Reviewed-by: kvn, shade, stuefe
diff --git a/src/hotspot/share/ci/ciKlass.hpp b/src/hotspot/share/ci/ciKlass.hpp
@@ -107,7 +107,7 @@ class ciKlass : public ciType {
   bool is_in_encoding_range() {
     Klass* k = get_Klass();
     bool is_in_encoding_range = CompressedKlassPointers::is_encodable(k);
-    assert(is_in_encoding_range || k->is_interface() || k->is_abstract(), "sanity");
+    assert(is_in_encoding_range, "sanity");
     return is_in_encoding_range;
   }
 
diff --git a/src/hotspot/share/classfile/classFileParser.cpp b/src/hotspot/share/classfile/classFileParser.cpp
@@ -5929,15 +5929,6 @@ bool ClassFileParser::is_java_lang_ref_Reference_subclass() const {
   return _super_klass->reference_type() != REF_NONE;
 }
 
-// Returns true if the future Klass will need to be addressable with a narrow Klass ID.
-bool ClassFileParser::klass_needs_narrow_id() const {
-  // Classes that are never instantiated need no narrow Klass Id, since the
-  // only point of having a narrow id is to put it into an object header. Keeping
-  // never instantiated classes out of class space lessens the class space pressure.
-  // For more details, see JDK-8338526.
-  return !is_interface() && !is_abstract();
-}
-
 // ----------------------------------------------------------------------------
 // debugging
 
diff --git a/src/hotspot/share/classfile/classFileParser.hpp b/src/hotspot/share/classfile/classFileParser.hpp
@@ -515,11 +515,6 @@ class ClassFileParser {
 
   bool is_hidden() const { return _is_hidden; }
   bool is_interface() const { return _access_flags.is_interface(); }
-  bool is_abstract() const { return _access_flags.is_abstract(); }
-
-  // Returns true if the Klass to be generated will need to be addressable
-  // with a narrow Klass ID.
-  bool klass_needs_narrow_id() const;
 
   ClassLoaderData* loader_data() const { return _loader_data; }
   const Symbol* class_name() const { return _class_name; }
diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp
@@ -89,7 +89,7 @@ DEBUG_ONLY(bool SystemDictionaryShared::_class_loading_may_happen = true;)
 #ifdef ASSERT
 static void check_klass_after_loading(const Klass* k) {
 #ifdef _LP64
-  if (k != nullptr && UseCompressedClassPointers && k->needs_narrow_id()) {
+  if (k != nullptr && UseCompressedClassPointers) {
     CompressedKlassPointers::check_encodable(k);
   }
 #endif
diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdKlassQueue.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdKlassQueue.cpp
@@ -29,6 +29,7 @@
 #include "jfr/support/jfrThreadLocal.hpp"
 #include "jfr/utilities/jfrEpochQueue.inline.hpp"
 #include "jfr/utilities/jfrTypes.hpp"
+#include "memory/metaspace.hpp"
 #include "oops/compressedKlass.inline.hpp"
 #include "utilities/macros.hpp"
 
@@ -73,14 +74,13 @@ static size_t element_size(bool compressed) {
   return compressed ? NARROW_ELEMENT_SIZE : ELEMENT_SIZE;
 }
 
-static bool can_compress_element(const Klass* klass) {
-  return CompressedKlassPointers::is_encodable(klass) &&
-         JfrTraceId::load_raw(klass) < uncompressed_threshold;
+static bool can_compress_element(traceid id) {
+  return Metaspace::using_class_space() && id < uncompressed_threshold;
 }
 
 static size_t element_size(const Klass* klass) {
   assert(klass != nullptr, "invariant");
-  return element_size(can_compress_element(klass));
+  return element_size(can_compress_element(JfrTraceId::load_raw(klass)));
 }
 
 static bool is_unloaded(traceid id, bool previous_epoch) {
@@ -136,8 +136,7 @@ static inline void store_traceid(JfrEpochQueueNarrowKlassElement* element, trace
 }
 
 static void store_compressed_element(traceid id, const Klass* klass, u1* pos) {
-  assert(can_compress_element(klass), "invariant");
-  assert(id == JfrTraceId::load_raw(klass), "invariant");
+  assert(can_compress_element(id), "invariant");
   JfrEpochQueueNarrowKlassElement* const element = new (pos) JfrEpochQueueNarrowKlassElement();
   store_traceid(element, id);
   element->compressed_klass = encode(klass);
@@ -153,7 +152,7 @@ static void store_element(const Klass* klass, u1* pos) {
   assert(pos != nullptr, "invariant");
   assert(klass != nullptr, "invariant");
   const traceid id = JfrTraceId::load_raw(klass);
-  if (can_compress_element(klass)) {
+  if (can_compress_element(id)) {
     store_compressed_element(id, klass, pos);
     return;
   }
diff --git a/src/hotspot/share/memory/allocation.cpp b/src/hotspot/share/memory/allocation.cpp
@@ -73,15 +73,15 @@ void* MetaspaceObj::operator new(size_t size, ClassLoaderData* loader_data,
                                  MetaspaceObj::Type type, TRAPS) throw() {
   // Klass has its own operator new
   assert(type != ClassType, "class has its own operator new");
-  return Metaspace::allocate(loader_data, word_size, type, /*use_class_space*/ false, THREAD);
+  return Metaspace::allocate(loader_data, word_size, type, THREAD);
 }
 
 void* MetaspaceObj::operator new(size_t size, ClassLoaderData* loader_data,
                                  size_t word_size,
                                  MetaspaceObj::Type type) throw() {
   assert(!Thread::current()->is_Java_thread(), "only allowed by non-Java thread");
   assert(type != ClassType, "class has its own operator new");
-  return Metaspace::allocate(loader_data, word_size, type, /*use_class_space*/ false);
+  return Metaspace::allocate(loader_data, word_size, type);
 }
 
 // This is used for allocating training data. We are allocating training data in many cases where a GC cannot be triggered.
diff --git a/src/hotspot/share/memory/metaspace.cpp b/src/hotspot/share/memory/metaspace.cpp
@@ -872,7 +872,7 @@ size_t Metaspace::max_allocation_word_size() {
 // is suitable for calling from non-Java threads.
 // Callers are responsible for checking null.
 MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
-                              MetaspaceObj::Type type, bool use_class_space) {
+                              MetaspaceObj::Type type) {
   assert(word_size <= Metaspace::max_allocation_word_size(),
          "allocation size too large (%zu)", word_size);
 
@@ -882,7 +882,7 @@ MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
   // Deal with concurrent unloading failed allocation starvation
   MetaspaceCriticalAllocation::block_if_concurrent_purge();
 
-  MetadataType mdtype = use_class_space ? ClassType : NonClassType;
+  MetadataType mdtype = (type == MetaspaceObj::ClassType) ? ClassType : NonClassType;
 
   // Try to allocate metadata.
   MetaWord* result = loader_data->metaspace_non_null()->allocate(word_size, mdtype);
@@ -906,18 +906,18 @@ MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
 }
 
 MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size,
-                              MetaspaceObj::Type type, bool use_class_space, TRAPS) {
+                              MetaspaceObj::Type type, TRAPS) {
 
   if (HAS_PENDING_EXCEPTION) {
     assert(false, "Should not allocate with exception pending");
     return nullptr;  // caller does a CHECK_NULL too
   }
   assert(!THREAD->owns_locks(), "allocating metaspace while holding mutex");
 
-  MetaWord* result = allocate(loader_data, word_size, type, use_class_space);
+  MetaWord* result = allocate(loader_data, word_size, type);
 
   if (result == nullptr) {
-    MetadataType mdtype = use_class_space ? ClassType : NonClassType;
+    MetadataType mdtype = (type == MetaspaceObj::ClassType) ? ClassType : NonClassType;
     tracer()->report_metaspace_allocation_failure(loader_data, word_size, type, mdtype);
 
     // Allocation failed.
diff --git a/src/hotspot/share/memory/metaspace.hpp b/src/hotspot/share/memory/metaspace.hpp
@@ -120,12 +120,12 @@ class Metaspace : public AllStatic {
   static constexpr size_t min_allocation_word_size = min_allocation_alignment_words;
 
   static MetaWord* allocate(ClassLoaderData* loader_data, size_t word_size,
-                            MetaspaceObj::Type type, bool use_class_space, TRAPS);
+                            MetaspaceObj::Type type, TRAPS);
 
   // Non-TRAPS version of allocate which can be called by a non-Java thread, that returns
   // null on failure.
   static MetaWord* allocate(ClassLoaderData* loader_data, size_t word_size,
-                            MetaspaceObj::Type type, bool use_class_space);
+                            MetaspaceObj::Type type);
 
   // Returns true if the pointer points into class space, non-class metaspace, or the
   // metadata portion of the CDS archive.
diff --git a/src/hotspot/share/oops/array.inline.hpp b/src/hotspot/share/oops/array.inline.hpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2025, 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
@@ -34,14 +34,14 @@ template <typename T>
 inline void* Array<T>::operator new(size_t size, ClassLoaderData* loader_data, int length, TRAPS) throw() {
   size_t word_size = Array::size(length);
   return (void*) Metaspace::allocate(loader_data, word_size,
-                                     MetaspaceObj::array_type(sizeof(T)), false, THREAD);
+                                     MetaspaceObj::array_type(sizeof(T)), THREAD);
 }
 
 template <typename T>
 inline void* Array<T>::operator new(size_t size, ClassLoaderData* loader_data, int length) throw() {
   size_t word_size = Array::size(length);
   return (void*) Metaspace::allocate(loader_data, word_size,
-                                     MetaspaceObj::array_type(sizeof(T)), false);
+                                     MetaspaceObj::array_type(sizeof(T)));
 }
 
 template <typename T>
diff --git a/src/hotspot/share/oops/arrayKlass.cpp b/src/hotspot/share/oops/arrayKlass.cpp
@@ -41,10 +41,6 @@
 #include "oops/oop.inline.hpp"
 #include "runtime/handles.inline.hpp"
 
-void* ArrayKlass::operator new(size_t size, ClassLoaderData* loader_data, size_t word_size, TRAPS) throw() {
-  return Metaspace::allocate(loader_data, word_size, MetaspaceObj::ClassType, true, THREAD);
-}
-
 ArrayKlass::ArrayKlass() {
   assert(CDSConfig::is_dumping_static_archive() || CDSConfig::is_using_archive(), "only for CDS");
 }
diff --git a/src/hotspot/share/oops/arrayKlass.hpp b/src/hotspot/share/oops/arrayKlass.hpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2025, 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
@@ -49,8 +49,6 @@ class ArrayKlass: public Klass {
   ArrayKlass(Symbol* name, KlassKind kind);
   ArrayKlass();
 
-  void* operator new(size_t size, ClassLoaderData* loader_data, size_t word_size, TRAPS) throw();
-
  public:
   // Testing operation
   DEBUG_ONLY(bool is_array_klass_slow() const { return true; })
diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp
@@ -455,11 +455,6 @@ const char* InstanceKlass::nest_host_error() {
   }
 }
 
-void* InstanceKlass::operator new(size_t size, ClassLoaderData* loader_data, size_t word_size,
-                                  bool use_class_space, TRAPS) throw() {
-  return Metaspace::allocate(loader_data, word_size, ClassType, use_class_space, THREAD);
-}
-
 InstanceKlass* InstanceKlass::allocate_instance_klass(const ClassFileParser& parser, TRAPS) {
   const int size = InstanceKlass::size(parser.vtable_size(),
                                        parser.itable_size(),
@@ -472,27 +467,26 @@ InstanceKlass* InstanceKlass::allocate_instance_klass(const ClassFileParser& par
   assert(loader_data != nullptr, "invariant");
 
   InstanceKlass* ik;
-  const bool use_class_space = UseClassMetaspaceForAllClasses || parser.klass_needs_narrow_id();
 
   // Allocation
   if (parser.is_instance_ref_klass()) {
     // java.lang.ref.Reference
-    ik = new (loader_data, size, use_class_space, THREAD) InstanceRefKlass(parser);
+    ik = new (loader_data, size, THREAD) InstanceRefKlass(parser);
   } else if (class_name == vmSymbols::java_lang_Class()) {
     // mirror - java.lang.Class
-    ik = new (loader_data, size, use_class_space, THREAD) InstanceMirrorKlass(parser);
+    ik = new (loader_data, size, THREAD) InstanceMirrorKlass(parser);
   } else if (is_stack_chunk_class(class_name, loader_data)) {
     // stack chunk
-    ik = new (loader_data, size, use_class_space, THREAD) InstanceStackChunkKlass(parser);
+    ik = new (loader_data, size, THREAD) InstanceStackChunkKlass(parser);
   } else if (is_class_loader(class_name, parser)) {
     // class loader - java.lang.ClassLoader
-    ik = new (loader_data, size, use_class_space, THREAD) InstanceClassLoaderKlass(parser);
+    ik = new (loader_data, size, THREAD) InstanceClassLoaderKlass(parser);
   } else {
     // normal
-    ik = new (loader_data, size, use_class_space, THREAD) InstanceKlass(parser);
+    ik = new (loader_data, size, THREAD) InstanceKlass(parser);
   }
 
-  if (ik != nullptr && UseCompressedClassPointers && use_class_space) {
+  if (ik != nullptr && UseCompressedClassPointers) {
     assert(CompressedKlassPointers::is_encodable(ik),
            "Klass " PTR_FORMAT "needs a narrow Klass ID, but is not encodable", p2i(ik));
   }
diff --git a/src/hotspot/share/oops/instanceKlass.hpp b/src/hotspot/share/oops/instanceKlass.hpp
@@ -143,8 +143,6 @@ class InstanceKlass: public Klass {
  protected:
   InstanceKlass(const ClassFileParser& parser, KlassKind kind = Kind, ReferenceType reference_type = REF_NONE);
 
-  void* operator new(size_t size, ClassLoaderData* loader_data, size_t word_size, bool use_class_space, TRAPS) throw();
-
  public:
   InstanceKlass();
 
diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp
@@ -279,18 +279,19 @@ static markWord make_prototype(const Klass* kls) {
 #ifdef _LP64
   if (UseCompactObjectHeaders) {
     // With compact object headers, the narrow Klass ID is part of the mark word.
-    // We therfore seed the mark word with the narrow Klass ID.
-    // Note that only those Klass that can be instantiated have a narrow Klass ID.
-    // For those who don't, we leave the klass bits empty and assert if someone
-    // tries to use those.
-    const narrowKlass nk = CompressedKlassPointers::is_encodable(kls) ?
-        CompressedKlassPointers::encode(const_cast<Klass*>(kls)) : 0;
+    // We therefore seed the mark word with the narrow Klass ID.
+    precond(CompressedKlassPointers::is_encodable(kls));
+    const narrowKlass nk = CompressedKlassPointers::encode(const_cast<Klass*>(kls));
     prototype = prototype.set_narrow_klass(nk);
   }
 #endif
   return prototype;
 }
 
+void* Klass::operator new(size_t size, ClassLoaderData* loader_data, size_t word_size, TRAPS) throw() {
+  return Metaspace::allocate(loader_data, word_size, MetaspaceObj::ClassType, THREAD);
+}
+
 Klass::Klass() : _kind(UnknownKlassKind) {
   assert(CDSConfig::is_dumping_static_archive() || CDSConfig::is_using_archive(), "only for cds");
 }
@@ -1060,7 +1061,7 @@ void Klass::verify_on(outputStream* st) {
   // This can be expensive, but it is worth checking that this klass is actually
   // in the CLD graph but not in production.
 #ifdef ASSERT
-  if (UseCompressedClassPointers && needs_narrow_id()) {
+  if (UseCompressedClassPointers) {
     // Stricter checks for both correct alignment and placement
     CompressedKlassPointers::check_encodable(this);
   } else {
diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp
@@ -207,6 +207,8 @@ class Klass : public Metadata {
   Klass(KlassKind kind);
   Klass();
 
+  void* operator new(size_t size, ClassLoaderData* loader_data, size_t word_size, TRAPS) throw();
+
  public:
   int kind() { return _kind; }
 
@@ -794,10 +796,6 @@ class Klass : public Metadata {
   static bool is_valid(Klass* k);
 
   static void on_secondary_supers_verification_failure(Klass* super, Klass* sub, bool linear_result, bool table_result, const char* msg);
-
-  // Returns true if this Klass needs to be addressable via narrow Klass ID.
-  inline bool needs_narrow_id() const;
-
 };
 
 #endif // SHARE_OOPS_KLASS_HPP
diff --git a/src/hotspot/share/oops/klass.inline.hpp b/src/hotspot/share/oops/klass.inline.hpp
@@ -175,13 +175,4 @@ inline bool Klass::search_secondary_supers(Klass *k) const {
   return result;
 }
 
-// Returns true if this Klass needs to be addressable via narrow Klass ID.
-inline bool Klass::needs_narrow_id() const {
-  // Classes that are never instantiated need no narrow Klass Id, since the
-  // only point of having a narrow id is to put it into an object header. Keeping
-  // never instantiated classes out of class space lessens the class space pressure.
-  // For more details, see JDK-8338526.
-  // Note: don't call this function before access flags are initialized.
-  return UseClassMetaspaceForAllClasses || (!is_abstract() && !is_interface());
-}
 #endif // SHARE_OOPS_KLASS_INLINE_HPP
diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp
@@ -2000,9 +2000,6 @@ const int ObjectAlignmentInBytes = 8;
           "Minimal number of elements in a sorted collection to prefer"     \
           "binary search over simple linear search." )                      \
                                                                             \
-  product(bool, UseClassMetaspaceForAllClasses, false, DIAGNOSTIC,          \
-          "Use the class metaspace for all classes including "              \
-          "abstract and interface classes.")                                \
 
 // end of RUNTIME_FLAGS
 
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotMetaspaceConstantImpl.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotMetaspaceConstantImpl.java
@@ -43,9 +43,6 @@ static MetaspaceObject getMetaspaceObject(Constant constant) {
     private HotSpotMetaspaceConstantImpl(MetaspaceObject metaspaceObject, boolean compressed) {
         this.metaspaceObject = metaspaceObject;
         this.compressed = compressed;
-        if (compressed && !canBeStoredInCompressibleMetaSpace()) {
-            throw new IllegalArgumentException("constant cannot be compressed: " + metaspaceObject);
-        }
     }
 
     @Override
@@ -88,19 +85,7 @@ public boolean isCompressed() {
 
     @Override
     public boolean isCompressible() {
-        if (compressed) {
-            return false;
-        }
-        return canBeStoredInCompressibleMetaSpace();
-    }
-
-    private boolean canBeStoredInCompressibleMetaSpace() {
-        if (!HotSpotVMConfig.config().useClassMetaspaceForAllClasses && metaspaceObject instanceof HotSpotResolvedJavaType t && !t.isArray()) {
-            // As of JDK-8338526, interface and abstract types are not stored
-            // in compressible metaspace.
-            return !t.isInterface() && !t.isAbstract();
-        }
-        return true;
+        return !compressed;
     }
 
     @Override
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotVMConfig.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotVMConfig.java
@@ -67,8 +67,6 @@ static String getHostArchitectureName() {
 
     final boolean useCompressedOops = getFlag("UseCompressedOops", Boolean.class);
 
-    final boolean useClassMetaspaceForAllClasses = getFlag("UseClassMetaspaceForAllClasses", Boolean.class);
-
     final int objectAlignment = getFlag("ObjectAlignmentInBytes", Integer.class);
 
     final int klassOffsetInBytes = getFieldValue("CompilerToVM::Data::oopDesc_klass_offset_in_bytes", Integer.class, "int");

Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ class ciKlass : public ciType {`
`107`	`107`	`bool is_in_encoding_range() {`
`108`	`108`	`Klass* k = get_Klass();`
`109`	`109`	`bool is_in_encoding_range = CompressedKlassPointers::is_encodable(k);`
`110`		`- assert(is_in_encoding_range \|\| k->is_interface() \|\| k->is_abstract(), "sanity");`
	`110`	`+ assert(is_in_encoding_range, "sanity");`
`111`	`111`	`return is_in_encoding_range;`
`112`	`112`	`}`
`113`	`113`
Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ DEBUG_ONLY(bool SystemDictionaryShared::_class_loading_may_happen = true;)`
`89`	`89`	`#ifdef ASSERT`
`90`	`90`	`static void check_klass_after_loading(const Klass* k) {`
`91`	`91`	`#ifdef _LP64`
`92`		`- if (k != nullptr && UseCompressedClassPointers && k->needs_narrow_id()) {`
	`92`	`+ if (k != nullptr && UseCompressedClassPointers) {`
`93`	`93`	`CompressedKlassPointers::check_encodable(k);`
`94`	`94`	`}`
`95`	`95`	`#endif`