RFR: 8358326: Use oopFactory array allocation

[Stefan Karlsson]

On Mon, 2 Jun 2025 14:07:10 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> This patch removes cases of direct calls to {type,obj}ArrayKlass->allocate() and calls oopFactory::new_*array instead.  It also renames {type,obj}ArrayKlass->allocate functions to allocate_klass and allocate_instance so it's more clear which allocation it's doing and to match InstanceKlass allocate functions, and makes these functions private with friends for Deoptimization and oopFactory.  For JEP 401, arrays are being extended to support new formats and attributes and this reduces the call sites.
> Tested with tier1-7.

This looks good to me. I a few suggestions that you could take if you want to.

src/hotspot/share/oops/objArrayKlass.hpp line 39:

> 37:   friend class JVMCIVMStructs;
> 38:   friend class oopFactory;
> 39:   friend class Deoptimization;

If you want you could consider sorting the friend declarations (here and in the other place where you added it)

src/hotspot/share/oops/objArrayKlass.hpp line 81:

> 79:                                                 int n, Klass* element_klass, TRAPS);
> 80: 
> 81:   objArrayOop allocate(int length, TRAPS);

Do you think `multi_allocate` will need a better name in the future?

src/hotspot/share/runtime/reflection.cpp line 352:

> 350:     if (type == T_VOID) {
> 351:       THROW_NULL(vmSymbols::java_lang_IllegalArgumentException());
> 352:     }

I was first wondering where this came from but I now see that this was duplicated from `basic_type_mirror_to_arrayklass`. I wonder if this could could be deduplicated by moving this check into `basic_type_mirror_to_basic_type` and then removed from -`basic_type_mirror_to_arrayklass`:


static BasicType basic_type_mirror_to_basic_type(oop basic_type_mirror, TRAPS) {
  assert(java_lang_Class::is_primitive(basic_type_mirror),
    "just checking");
  
  if (type == T_VOID) {
    THROW_NULL(vmSymbols::java_lang_IllegalArgumentException());
  }

  return java_lang_Class::primitive_type(basic_type_mirror);
}

static Klass* basic_type_mirror_to_arrayklass(oop basic_type_mirror, TRAPS) {
  BasicType type = basic_type_mirror_to_basic_type(basic_type_mirror);
  return Universe::typeArrayKlass(type);
}

And then this code could be a two-liner again:

if (java_lang_Class::is_primitive(element_mirror)) {
    BasicType type = basic_type_mirror_to_basic_type(element_mirror);
  return oopFactory::new_typeArray(type, length, CHECK_NULL);
}

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25590#pullrequestreview-2909210112
PR Review Comment: https://git.openjdk.org/jdk/pull/25590#discussion_r2135207589
PR Review Comment: https://git.openjdk.org/jdk/pull/25590#discussion_r2135211890
PR Review Comment: https://git.openjdk.org/jdk/pull/25590#discussion_r2135202824