RFR: 8365823: Revert storing abstract and interface Klasses to non-class metaspace [v3]

Aleksey Shipilev shade at openjdk.org
Mon Sep 15 15:09:38 UTC 2025


On Mon, 15 Sep 2025 14:38:03 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change removes the optimization to not store abstract and interface Klass metadata to non-class metaspace.  Now all Klass metadata is in the Klass metaspace.  This is simpler and less bug prone, and didn't help with the limitation of classes that can be stored in class metaspace materially.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove CFP.is_abstract().

This looks good to me, I have a few more questions:

src/hotspot/share/ci/ciKlass.hpp line 112:

> 110:     assert(is_in_encoding_range, "sanity");
> 111:     return is_in_encoding_range;
> 112:   }

I thought about this method, and it _looks_ to be just:


  bool is_in_encoding_range() {
    assert(CompressedKlassPointers::is_encodable(get_Klass()), "sanity");
    return true;
  }


But I think your version is better, since it is more paranoid and actually checks if we are dealing with the class within the class range.

src/hotspot/share/memory/allocation.cpp line 76:

> 74:   // Klass has its own operator new
> 75:   assert(type != ClassType, "class has its own operator new");
> 76:   return Metaspace::allocate(loader_data, word_size, type, /*use_class_space*/ false, THREAD);

Question: now that `ArrayKlass` and `InstanceKlass` do not have `::new`, the assert above is still valid? I am guessing all these inherit `Klass::new`?

-------------

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27295#pullrequestreview-3225077178
PR Review Comment: https://git.openjdk.org/jdk/pull/27295#discussion_r2349317958
PR Review Comment: https://git.openjdk.org/jdk/pull/27295#discussion_r2349286698


More information about the graal-dev mailing list