On Fri, 13 Feb 2026 15:47:19 GMT, Stefan Karlsson <[email protected]> wrote:

>> Frederic Parain has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix search of specific array klass
>
> src/hotspot/share/memory/oopFactory.cpp line 127:
> 
>> 125:   // Cast below must pass because the array description required a 
>> RefArrayKlass
>> 126:   RefArrayKlass* rak = RefArrayKlass::cast(oak);
>> 127:   oop array = rak->allocate_instance(length, properties, CHECK_NULL);
> 
> While looking at why we have all three 
> `[Obj|Ref|Flat]ArrayKlass::allocate_instance` functions I realize that this 
> will most likely make a virtual call to `ObjArrayKlass::allocate_instance`. 
> If you want to call directly into `RefArrayKlass` this could be changed to:
> 
> Suggestion:
> 
>   oop array = rak->RefArrayKlass::allocate_instance(length, properties, 
> CHECK_NULL);
> 
> 
> This is probably not super important, but I wanted to mention it because this 
> was not obvious to me upon first reading of this function.

Call updated to improve readability.

> src/hotspot/share/prims/jvm.cpp line 2046:
> 
>> 2044:     refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(),
>> 2045:                                                      length,
>> 2046:                                                      CHECK_NULL);
> 
> Suggestion:
> 
>     refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(), 
> length, CHECK_NULL);
> 
> or:
> 
> Suggestion:
> 
>     refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(),
>                                              length,
>                                              CHECK_NULL);

Fixed

> src/hotspot/share/services/management.cpp line 1469:
> 
>> 1467:     refArrayOop res = 
>> oopFactory::new_refArray(vmClasses::String_klass(),
>> 1468:                                                        num_entries,
>> 1469:                                                        CHECK_NULL);
> 
> Suggestion:
> 
>     refArrayOop res = oopFactory::new_refArray(vmClasses::String_klass(), 
> num_entries, CHECK_NULL);
> 
> or:
> 
> Suggestion:
> 
>     refArrayOop res = oopFactory::new_refArray(vmClasses::String_klass(),
>                                                num_entries,
>                                                CHECK_NULL);

Fixed

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2033#discussion_r2805613761
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2033#discussion_r2805609597
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2033#discussion_r2805612524

Reply via email to