RFR: 8349088: De-virtualize Codeblob and nmethod [v2]

[Chris Plummer]

On Sun, 9 Feb 2025 19:43:29 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Remove virtual methods from CodeBlob and nmethod to simplify saving/restoring in Leyden AOT cache. It avoids the need to patch hidden VPTR pointer to class's virtual table.
>> 
>> Added C++ static asserts to make sure no virtual methods are added in a future.
>> 
>> Fixed/cleaned SA code which process CodeBlob and its subclasses. Use `CodeBlob::_kind` field value to determine the type of blob.
>> 
>> Tested tier1-5, hs-tier6-rt (for JFR testing), stress, xcomp
>
> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix Zero and Minimal VM builds

I almost wished I hadn't looked because there is a lot of SA CodeBlob support that could use some cleanup. Most notably I think most of the wrapper subclasses are not needed by SA, and could be served by one common class. See what I'm doing in #23456 for JavaThread subclasses. Wrapper classes don't need to be 1-to-1 with the class type they are wrapping. A single wrapper class type can handle any number of hotspot types. It usually just make more sense for them to be 1-to-1, but when they are trivial and the implementation is replicated across subtypes, just having one wrapper class implement them all can simplify things.

The other thing I noticed is a lot of the subtypes seem to be doing some unnecessary things like registering Observers, and they all do something like the following:

44     Type type = db.lookupType("BufferBlob");

Even when it never references "type".

I'm not suggesting you clean up any of this now, but just pointed it out. I might file an issue and try to clean it up myself at some point.

I still need to take a closer look at the SA changes.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CodeCache.java line 38:

> 36: public class CodeCache {
> 37:   private static GrowableArray<CodeHeap> heapArray;
> 38:   private static VirtualConstructor virtualConstructor;

What is the reason for switching from the virtualConstructor/hashMap approach to using getClassFor()? The hashmap is the model for JavaThread, MetaData, and CollectedHeap subtypes.

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

PR Review: https://git.openjdk.org/jdk/pull/23533#pullrequestreview-2604594200
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1948335278