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

[Vladimir Kozlov]

On Thu, 13 Feb 2025 17:22:18 GMT, John R Rose <jrose at openjdk.org> wrote:

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

Thank you, @rose00 and @xmas92, for review and suggestions.

Let me say it first - printing code for code blobs and nmethod is big mess. It requires separate big change to clean it up.
For example, I have to go through CodeBlob's virtual dispatch `print_value_on_v()` for nmethod because some sets of `nmethod::print*()` are defined only in debug VM: [nmethod.hpp#L919](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/nmethod.hpp#L919)

Then `nmethod` has other mess which requires C++ trickery because it does not follow print API in CodeBlob:

  void print(outputStream* st) const;
  // need to re-define this from CodeBlob else the overload hides it
  void print_on(outputStream* st) const override { CodeBlob::print_on(st); }
  void print_on(outputStream* st, const char* msg) const;

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

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