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

[Vladimir Kozlov]

On Thu, 13 Feb 2025 08:15:16 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix Zero VM build
>
> src/hotspot/share/code/codeBlob.hpp line 140:
> 
>> 138:       instance->print_value_on_nv(st);
>> 139:     }
>> 140:   };
> 
> I wonder why the base class is not abstract. AFAICT `print_value_on` is unreachable and `print_on` is only used by `DeoptimizationBlob::Vptr` which also seems like a behavioural change, as before this patch calling `print_on` a `DeoptimizationBlob` object would dispatch to `SingletonBlob::print_on` not `CodeBlob::print_on`. 
> 
> Suggestion:
> 
>  struct Vptr {
>     virtual void print_on(const CodeBlob* instance, outputStream* st) const = 0;
>     virtual void print_value_on(const CodeBlob* instance, outputStream* st) const = 0;
>   };

done

> src/hotspot/share/code/codeBlob.hpp line 339:
> 
>> 337:   void print_value_on(outputStream* st) const;
>> 338: 
>> 339:   class Vptr : public CodeBlob::Vptr {
> 
> I wonder if these should share the same type hierarchy as tier container class. This would also solve the issueI noted in my other comment about not calling the correct `print_on`.
> Suggestion:
> 
>   class Vptr : public RuntimeBlob::Vptr {

Fixed

> src/hotspot/share/code/codeBlob.hpp line 427:
> 
>> 425:   void print_value_on(outputStream* st) const;
>> 426: 
>> 427:   class Vptr : public CodeBlob::Vptr {
> 
> Suggestion:
> 
>   class Vptr : public RuntimeBlob::Vptr {

Fixed

> src/hotspot/share/code/codeBlob.hpp line 467:
> 
>> 465:   void print_value_on(outputStream* st) const;
>> 466: 
>> 467:   class Vptr : public CodeBlob::Vptr {
> 
> Suggestion:
> 
>   class Vptr : public RuntimeBlob::Vptr {

Fixed

> src/hotspot/share/code/codeBlob.hpp line 553:
> 
>> 551:   void print_value_on(outputStream* st) const;
>> 552: 
>> 553:   class Vptr : public CodeBlob::Vptr {
> 
> This one specifically
> Suggestion:
> 
>   class Vptr : public SingletonBlob::Vptr {

fixed

> src/hotspot/share/code/codeBlob.hpp line 679:
> 
>> 677:   void print_value_on(outputStream* st) const;
>> 678: 
>> 679:   class Vptr : public CodeBlob::Vptr {
> 
> Suggestion:
> 
>   class Vptr : public RuntimeBlob::Vptr {

fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1956799673
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1956801833
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1956801994
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1956802109
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1956803039
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1956827486