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

[John R Rose]

On Thu, 13 Feb 2025 17:14:59 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:
> 
>   rename SA argument

One related idea:  The Vptr classes seem to be regular enough to be templated.  That is, one class body, instantiated with a template argument for each code blob type (and probably another for the enum).  That would remove some of the distracting per-class boilerplate.  Something like:


template<typename CB_T, CodeBlob::Kind Tkind>
class Vptr_Impl : public Vptr {
  override void print_on(const CodeBlob* instance, outputStream* st) const {
    assert(instance->kind() == Tkind, "sanity");
    ((const CB_T*)instance)->print_on_impl(st);
  }
  …
  override bool assert_sane(cosnt CodeBlob* instance) {
     assert(instance->kind() == Tkind, "");
     return true;
  }
};

class CodeBlob {
  public:
  final Vptr* vptr() const {
    Vptr* vptr = vptr_array[_kind];
    assert(vptr->assert_sant(this), "correct array element");
    return vptr;
  }
  final void print_on(outputStream* st) const {
    vptr()->print_on(this, st);
  }
};


Then:


const Vptr* array[] = {
  &Vptr_Impl<CodeBlob, CodeBlobKind>(),
  ...
  &Vptr_Impl<UncommonTrapBlob, UncommonTrapKind>(),
  ...
};


The array could be filled by a macro that tracks the enum members; I like that as a small job for macros (no code in it).

Then:


class UncommonTrapBlob : public OtherBlob {
  protected:  // impl "M" method is not public
  void print_on_impl(outputStream* st) const {
    OtherBlob::print_on_impl(st);
    st->print("my field = %d", _my_field);
  }
  // Vptr needs to call impl method
  friend class Vptr_Impl;  // this might break down, so make it all public in the end
};


I don't see any reason the Vptr subclasses need to be related in any more detail as subs or supers.

Well, C++ is a bag of surprises, so there are probably several reasons the above sketch is wrong.  But something like it might add a little more readability and predictability to the code.

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

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