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