On Thu, 13 Feb 2025 08:15:16 GMT, Axel Boldt-Christmas <[email protected]>
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
