On Thu, 13 Feb 2025 17:22:18 GMT, John R Rose <jr...@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
