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

[Vladimir Kozlov]

On Mon, 10 Feb 2025 03:11:22 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> 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.

Before I forgot to answer you, @plummercj 
I completely agree with your comment about cleaning up wrapper subclasses which do nothing.

I think some wrapper subclasses for CodeBlob were kept because of `is*()` which were used only in `PStack` to print name. Why not use `getName()` for this purpose without big `if/else` there?

An other purpose could be a place holder for additional information in a future which never come.

Other wrapper provides information available in `CodeBlob`. Like `RuntimeStub. callerMustGCArguments()`. `_caller_must_gc_arguments` field is part of  VM's `CodeBlob` class for some time now. Looks like I missed change in SA when did change in VM.

So yes, feel free to clean this up.  I will help with review.

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

PR Comment: https://git.openjdk.org/jdk/pull/23533#issuecomment-2652321179