RFR: 8367989: Remove InstanceKlass::allocate_objArray

[Coleen Phillimore]

On Fri, 19 Sep 2025 08:15:20 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> This change removes InstanceKlass::allocate_objArray and has its caller call ObjArrayKlass::allocate_instance directly from oopFactory, like the other array allocations do.  See CR for more information why we should have this change.  I also removed element_klass_addr() and moved element_klass_offset() to be in a more logical place near element_klass() functions.  This upstreams a tiny valhalla diff.
>> Tested with tier1-4.
>
> src/hotspot/share/memory/oopFactory.cpp line 112:
> 
>> 110:   } else {
>> 111:     ArrayKlass* ak = InstanceKlass::cast(klass)->array_klass(CHECK_NULL);
>> 112:     return ObjArrayKlass::cast(ak)->allocate_instance(length, THREAD);
> 
> Before this change the two if/else branches had a symmetry that is lost with the proposed change. It makes you look at the code and wonder what the reason is for this asymmetry. Could the symmetry be retained by changing this to:
> 
> 
> objArrayOop oopFactory::new_objArray(Klass* klass, int length, TRAPS) {
>   ArrayKlass* ak;
> 
>   if (klass->is_array_klass()) {
>     ak = ArrayKlass::cast(klass)->array_klass(CHECK_NULL);
>   } else {
>     ak = InstanceKlass::cast(klass)->array_klass(CHECK_NULL);
>   }
>   
>   return ak->allocate_instance(length, THREAD);
> }
> 
> 
> Or if you dare to use a virtual call instead of the if branch:
> 
> objArrayOop oopFactory::new_objArray(Klass* klass, int length, TRAPS) {
>   ArrayKlass* ak = klass->array_klass(CHECK_NULL);
>   return ak->allocate_instance(length, THREAD);
> }
> 
> 
> If the virtual call is unwanted then we could add a new "faster" (unclear how much faster this actually is):
> 
> ArrayKlass* Klass::array_klass_fast(TRAPS) {
>   ArrayKlass* ak;
> 
>   if (klass->is_array_klass()) {
>     ak = ArrayKlass::cast(klass)->array_klass(CHECK_NULL);
>   } else {
>     ak = InstanceKlass::cast(klass)->array_klass(CHECK_NULL);
>   }
>  
>   assert(ak == array_klass(), "The two functions should return the same result");
>   return ak;
> }
> ...
> objArrayOop oopFactory::new_objArray(Klass* klass, int length, TRAPS) {
>   ArrayKlass* ak = klass->array_klass_fast(CHECK_NULL);
>   return ak->allocate_instance(length, THREAD);
> }
> 
> 
> (I've not compiled nor tested the above)

It wasn't really symmetric except number of lines.  One branch calls ArrayKlass::allocate_arrayArray, the other calls allocate_instance for ObjArray.  Unless I refactor allocate_arrayArray into this, it still won't be symmetrical, and it's quite a bit different in the valhalla repo.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27372#discussion_r2362599079