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

[Andrew Dinn]

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

src/hotspot/share/code/codeBlob.cpp line 58:

> 56: #include <type_traits>
> 57: 
> 58: // Virtual methods are not allowed in code blobs to simplify caching compiled code.

Is it worth considering generating this code plus also some of the existing code in the header using an iterator template macro? e.g.

    #define CODEBLOBS_DO(do_codeblob_abstract, do_codeblob_nonleaf, \
                         do_codeblob_leaf)                          \
      do_codeblob_abstract(CodeBlob)                                \
      do_codeblob_leaf(nmethod, Nmethod, nmethod)                   \
      do_codeblob_abstract(RuntimeBlob)                             \
      do_codeblob_nonleaf(BufferBlob, Buffer, buffer)               \
      do_codeblob_leaf(AdapterBlob, Adapter, adapter)               \
      . . .                                                         \
      do_codeblob_leaf(RuntimeStub, Runtime_Stub, runtime_stub)     \
      . . .

The macro arguments to the templates would themselves be macros:

    do_codeblob_abstract(classname) // abstract, non-instantiable class
    do_codeblob_nonleaf(classname, kindname, accessorname) // instantiable, subclassable
    do_codeblob_leaf(classname, kindname, accessorname) // instantiable, non-subclassable

Using a template macro like this to generate the code below -- *plus also* some of the code currently declared piecemeal in the header -- would guarantee all cases are covered now and will remain so later so when the macro is updated. I think it would probably also allow case handling code in AOT cache code to be generated.

So, we would generate the code here as follows

    #define EMPTY1(classname) 
    #define EMPTY3(classname, kindname, accessorname)

    #define assert_nonvirtual_leaf(classname, kindname, accessorname) \
      static_assert(!std::is_polymorphic<classname>::value,            \
                    "no virtual methods are allowed in " # classname );

    CODEBLOBS_DO(empty1, empty3, assert_nonvirtual_leaf)

    #undef assert_nonvirtual_leaf

Likewise in codeBlob.hpp we could generate `enum CodeBlobKind` to cover all the non-abstract classes and likewise generate the accessor methods `is_nmethod()`, `is_buffer_blob()` in class `CodeBlob` which allow the kind to be tested.

    #define codekind_enum_tag(classname, kindname, accessorname) \
      kindname,

    enum CodeBlobKind : u1 {
      None,
      CODEBLOBS_DO(empty1, codekind_enum_tag, codekind_enum_tag)
      Number_Of_Kinds
    };

    #define is_codeblob_define(classname, kindname, accessorname) \
        void is_ # accessor_name () { return _kind == kindname; }

    class CodeBlob {
      . . .
      CODEBLOBS_DO(empty1, is_codeblob_define, is_codeblob_define);
      . . .
There may be other opportunities to use the iterator (e.g. in vmStructs.cpp?) but this looks like a good start.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1948849392