On Sun, 9 Feb 2025 19:43:29 GMT, Vladimir Kozlov <k...@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
