Skip to content

8358326: Use oopFactory array allocation #25590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

8358326: Use oopFactory array allocation #25590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2da2b11
8358326: Use oopFactory array allocation
coleenp Jun 2, 2025
2ccaf42
Sort our friends.
coleenp Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/hotspot/share/jvmci/jvmciEnv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1463,8 +1463,7 @@ JVMCIPrimitiveArray JVMCIEnv::new_byteArray(int length, JVMCI_TRAPS) {
JVMCIObjectArray JVMCIEnv::new_byte_array_array(int length, JVMCI_TRAPS) {
JavaThread* THREAD = JavaThread::current(); // For exception macros.
if (is_hotspot()) {
Klass* byteArrayArrayKlass = TypeArrayKlass::cast(Universe::byteArrayKlass())->array_klass(CHECK_(JVMCIObject()));
objArrayOop result = ObjArrayKlass::cast(byteArrayArrayKlass) ->allocate(length, CHECK_(JVMCIObject()));
objArrayOop result = oopFactory::new_objArray(Universe::byteArrayKlass(), length, CHECK_(JVMCIObject()));
return wrap(result);
} else {
JNIAccessMark jni(this, THREAD);
Expand Down
20 changes: 10 additions & 10 deletions src/hotspot/share/memory/oopFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,41 +40,41 @@
#include "utilities/utf8.hpp"

typeArrayOop oopFactory::new_boolArray(int length, TRAPS) {
return Universe::boolArrayKlass()->allocate(length, THREAD);
return Universe::boolArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_charArray(int length, TRAPS) {
return Universe::charArrayKlass()->allocate(length, THREAD);
return Universe::charArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_floatArray(int length, TRAPS) {
return Universe::floatArrayKlass()->allocate(length, THREAD);
return Universe::floatArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_doubleArray(int length, TRAPS) {
return Universe::doubleArrayKlass()->allocate(length, THREAD);
return Universe::doubleArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_byteArray(int length, TRAPS) {
return Universe::byteArrayKlass()->allocate(length, THREAD);
return Universe::byteArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_shortArray(int length, TRAPS) {
return Universe::shortArrayKlass()->allocate(length, THREAD);
return Universe::shortArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_intArray(int length, TRAPS) {
return Universe::intArrayKlass()->allocate(length, THREAD);
return Universe::intArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_longArray(int length, TRAPS) {
return Universe::longArrayKlass()->allocate(length, THREAD);
return Universe::longArrayKlass()->allocate_instance(length, THREAD);
}

// create java.lang.Object[]
objArrayOop oopFactory::new_objectArray(int length, TRAPS) {
assert(Universe::objectArrayKlass() != nullptr, "Too early?");
return Universe::objectArrayKlass()->allocate(length, THREAD);
return Universe::objectArrayKlass()->allocate_instance(length, THREAD);
}

typeArrayOop oopFactory::new_charArray(const char* utf8_str, TRAPS) {
Expand All @@ -88,7 +88,7 @@ typeArrayOop oopFactory::new_charArray(const char* utf8_str, TRAPS) {

typeArrayOop oopFactory::new_typeArray(BasicType type, int length, TRAPS) {
TypeArrayKlass* klass = Universe::typeArrayKlass(type);
return klass->allocate(length, THREAD);
return klass->allocate_instance(length, THREAD);
}

// Create a Java array that points to Symbol.
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/oops/objArrayKlass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#include "runtime/mutexLocker.hpp"
#include "utilities/macros.hpp"

ObjArrayKlass* ObjArrayKlass::allocate(ClassLoaderData* loader_data, int n, Klass* k, Symbol* name, TRAPS) {
ObjArrayKlass* ObjArrayKlass::allocate_klass(ClassLoaderData* loader_data, int n, Klass* k, Symbol* name, TRAPS) {
assert(ObjArrayKlass::header_size() <= InstanceKlass::header_size(),
"array klasses must be same size as InstanceKlass");

Expand Down Expand Up @@ -100,7 +100,7 @@ ObjArrayKlass* ObjArrayKlass::allocate_objArray_klass(ClassLoaderData* loader_da
}

// Initialize instance variables
ObjArrayKlass* oak = ObjArrayKlass::allocate(loader_data, n, element_klass, name, CHECK_NULL);
ObjArrayKlass* oak = ObjArrayKlass::allocate_klass(loader_data, n, element_klass, name, CHECK_NULL);

ModuleEntry* module = oak->module();
assert(module != nullptr, "No module entry for array");
Expand Down Expand Up @@ -149,7 +149,7 @@ size_t ObjArrayKlass::oop_size(oop obj) const {
return objArrayOop(obj)->object_size();
}

objArrayOop ObjArrayKlass::allocate(int length, TRAPS) {
objArrayOop ObjArrayKlass::allocate_instance(int length, TRAPS) {
check_array_allocation_length(length, arrayOopDesc::max_array_length(T_OBJECT), CHECK_NULL);
size_t size = objArrayOopDesc::object_size(length);
return (objArrayOop)Universe::heap()->array_allocate(this, size, length,
Expand All @@ -160,7 +160,7 @@ oop ObjArrayKlass::multi_allocate(int rank, jint* sizes, TRAPS) {
int length = *sizes;
ArrayKlass* ld_klass = lower_dimension();
// If length < 0 allocate will throw an exception.
objArrayOop array = allocate(length, CHECK_NULL);
objArrayOop array = allocate_instance(length, CHECK_NULL);
objArrayHandle h_array (THREAD, array);
if (rank > 1) {
if (length != 0) {
Expand Down
9 changes: 6 additions & 3 deletions src/hotspot/share/oops/objArrayKlass.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ class ClassLoaderData;
// ObjArrayKlass is the klass for objArrays

class ObjArrayKlass : public ArrayKlass {
friend class VMStructs;
friend class Deoptimization;
friend class JVMCIVMStructs;
friend class oopFactory;
friend class VMStructs;

public:
static const KlassKind Kind = ObjArrayKlassKind;
Expand All @@ -47,7 +49,9 @@ class ObjArrayKlass : public ArrayKlass {

// Constructor
ObjArrayKlass(int n, Klass* element_klass, Symbol* name);
static ObjArrayKlass* allocate(ClassLoaderData* loader_data, int n, Klass* k, Symbol* name, TRAPS);
static ObjArrayKlass* allocate_klass(ClassLoaderData* loader_data, int n, Klass* k, Symbol* name, TRAPS);

objArrayOop allocate_instance(int length, TRAPS);
public:
// For dummy objects
ObjArrayKlass() {}
Expand Down Expand Up @@ -78,7 +82,6 @@ class ObjArrayKlass : public ArrayKlass {
static ObjArrayKlass* allocate_objArray_klass(ClassLoaderData* loader_data,
int n, Klass* element_klass, TRAPS);

objArrayOop allocate(int length, TRAPS);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought if changing multi_allocate_instance so that it's clear that it's an instance, but decided to limit this. Maybe this would be helpful but the allocate() function was the most confusing to me, that's why I picked that one.

oop multi_allocate(int rank, jint* sizes, TRAPS);

// Copying
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/oops/typeArrayKlass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ TypeArrayKlass* TypeArrayKlass::create_klass(BasicType type,

ClassLoaderData* null_loader_data = ClassLoaderData::the_null_class_loader_data();

TypeArrayKlass* ak = TypeArrayKlass::allocate(null_loader_data, type, sym, CHECK_NULL);
TypeArrayKlass* ak = TypeArrayKlass::allocate_klass(null_loader_data, type, sym, CHECK_NULL);

// Call complete_create_array_klass after all instance variables have been initialized.
complete_create_array_klass(ak, ak->super(), ModuleEntryTable::javabase_moduleEntry(), CHECK_NULL);
Expand All @@ -65,7 +65,7 @@ TypeArrayKlass* TypeArrayKlass::create_klass(BasicType type,
return ak;
}

TypeArrayKlass* TypeArrayKlass::allocate(ClassLoaderData* loader_data, BasicType type, Symbol* name, TRAPS) {
TypeArrayKlass* TypeArrayKlass::allocate_klass(ClassLoaderData* loader_data, BasicType type, Symbol* name, TRAPS) {
assert(TypeArrayKlass::header_size() <= InstanceKlass::header_size(),
"array klasses must be same size as InstanceKlass");

Expand Down Expand Up @@ -101,7 +101,7 @@ oop TypeArrayKlass::multi_allocate(int rank, jint* last_size, TRAPS) {
// For typeArrays this is only called for the last dimension
assert(rank == 1, "just checking");
int length = *last_size;
return allocate(length, THREAD);
return allocate_instance(length, THREAD);
}


Expand Down
9 changes: 6 additions & 3 deletions src/hotspot/share/oops/typeArrayKlass.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class ClassLoaderData;
// It contains the type and size of the elements

class TypeArrayKlass : public ArrayKlass {
friend class Deoptimization;
friend class oopFactory;
friend class VMStructs;

public:
Expand All @@ -43,7 +45,10 @@ class TypeArrayKlass : public ArrayKlass {

// Constructor
TypeArrayKlass(BasicType type, Symbol* name);
static TypeArrayKlass* allocate(ClassLoaderData* loader_data, BasicType type, Symbol* name, TRAPS);
static TypeArrayKlass* allocate_klass(ClassLoaderData* loader_data, BasicType type, Symbol* name, TRAPS);

typeArrayOop allocate_common(int length, bool do_zero, TRAPS);
typeArrayOop allocate_instance(int length, TRAPS) { return allocate_common(length, true, THREAD); }
public:
TypeArrayKlass() {} // For dummy objects.

Expand All @@ -66,8 +71,6 @@ class TypeArrayKlass : public ArrayKlass {
size_t oop_size(oop obj) const;

// Allocation
typeArrayOop allocate_common(int length, bool do_zero, TRAPS);
typeArrayOop allocate(int length, TRAPS) { return allocate_common(length, true, THREAD); }
oop multi_allocate(int rank, jint* sizes, TRAPS);

oop protection_domain() const { return nullptr; }
Expand Down
8 changes: 5 additions & 3 deletions src/hotspot/share/prims/jni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2285,9 +2285,11 @@ JNI_ENTRY(jobjectArray, jni_NewObjectArray(JNIEnv *env, jsize length, jclass ele
jobjectArray ret = nullptr;
DT_RETURN_MARK(NewObjectArray, jobjectArray, (const jobjectArray&)ret);
Klass* ek = java_lang_Class::as_Klass(JNIHandles::resolve_non_null(elementClass));
Klass* ak = ek->array_klass(CHECK_NULL);
ObjArrayKlass::cast(ak)->initialize(CHECK_NULL);
objArrayOop result = ObjArrayKlass::cast(ak)->allocate(length, CHECK_NULL);

// Make sure bottom_klass is initialized.
ek->initialize(CHECK_NULL);
objArrayOop result = oopFactory::new_objArray(ek, length, CHECK_NULL);

oop initial_value = JNIHandles::resolve(initialElement);
if (initial_value != nullptr) { // array already initialized with null
for (int index = 0; index < length; index++) {
Expand Down
5 changes: 2 additions & 3 deletions src/hotspot/share/prims/vectorSupport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "code/location.hpp"
#include "jni.h"
#include "jvm.h"
#include "memory/oopFactory.hpp"
#include "oops/klass.inline.hpp"
#include "oops/typeArrayOop.inline.hpp"
#include "prims/vectorSupport.hpp"
Expand Down Expand Up @@ -109,9 +110,7 @@ Handle VectorSupport::allocate_vector_payload_helper(InstanceKlass* ik, frame* f
int elem_size = type2aelembytes(elem_bt);

// On-heap vector values are represented as primitive arrays.
TypeArrayKlass* tak = Universe::typeArrayKlass(elem_bt);

typeArrayOop arr = tak->allocate(num_elem, CHECK_NH); // safepoint
typeArrayOop arr = oopFactory::new_typeArray(elem_bt, num_elem, CHECK_NH); // safepoint

if (location.is_register()) {
// Value was in a callee-saved register.
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/runtime/deoptimization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1274,11 +1274,11 @@ bool Deoptimization::realloc_objects(JavaThread* thread, frame* fr, RegisterMap*
assert(sv->field_size() % type2size[ak->element_type()] == 0, "non-integral array length");
int len = sv->field_size() / type2size[ak->element_type()];
InternalOOMEMark iom(THREAD);
obj = ak->allocate(len, THREAD);
obj = ak->allocate_instance(len, THREAD);
} else if (k->is_objArray_klass()) {
ObjArrayKlass* ak = ObjArrayKlass::cast(k);
InternalOOMEMark iom(THREAD);
obj = ak->allocate(sv->field_size(), THREAD);
obj = ak->allocate_instance(sv->field_size(), THREAD);
}

if (obj == nullptr) {
Expand Down
25 changes: 14 additions & 11 deletions src/hotspot/share/runtime/reflection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,16 @@ void Reflection::array_set(jvalue* value, arrayOop a, int index, BasicType value
}
}


// Conversion
static BasicType basic_type_mirror_to_basic_type(oop basic_type_mirror) {
assert(java_lang_Class::is_primitive(basic_type_mirror),
"just checking");
return java_lang_Class::primitive_type(basic_type_mirror);
}

static Klass* basic_type_mirror_to_arrayklass(oop basic_type_mirror, TRAPS) {
assert(java_lang_Class::is_primitive(basic_type_mirror), "just checking");
BasicType type = java_lang_Class::primitive_type(basic_type_mirror);
BasicType type = basic_type_mirror_to_basic_type(basic_type_mirror);
if (type == T_VOID) {
THROW_NULL(vmSymbols::java_lang_IllegalArgumentException());
}
Expand All @@ -339,8 +346,11 @@ arrayOop Reflection::reflect_new_array(oop element_mirror, jint length, TRAPS) {
THROW_MSG_NULL(vmSymbols::java_lang_NegativeArraySizeException(), err_msg("%d", length));
}
if (java_lang_Class::is_primitive(element_mirror)) {
Klass* tak = basic_type_mirror_to_arrayklass(element_mirror, CHECK_NULL);
return TypeArrayKlass::cast(tak)->allocate(length, THREAD);
BasicType type = basic_type_mirror_to_basic_type(element_mirror);
if (type == T_VOID) {
THROW_NULL(vmSymbols::java_lang_IllegalArgumentException());
}
Comment on lines +350 to +352
Copy link
Member

@stefank stefank Jun 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, CHECK_NULL);
  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, CHECK_NULL);
  return oopFactory::new_typeArray(type, length, CHECK_NULL);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the caller to basic_type_mirror_to_basic_type() can legitimately return T_VOID for the caller in reflect_method, so that's why I had to duplicate the exception code. Maybe a future enhancement would be to move these to javaClasses.hpp in java_lang_Class, where it knows all about is_primitive types, and boxing classes, which I guess boxing T_VOID is a thing.

return oopFactory::new_typeArray(type, length, CHECK_NULL);
} else {
Klass* k = java_lang_Class::as_Klass(element_mirror);
if (k->is_array_klass() && ArrayKlass::cast(k)->dimension() >= MAX_DIM) {
Expand Down Expand Up @@ -907,13 +917,6 @@ static methodHandle resolve_interface_call(InstanceKlass* klass,
return methodHandle(THREAD, info.selected_method());
}

// Conversion
static BasicType basic_type_mirror_to_basic_type(oop basic_type_mirror) {
assert(java_lang_Class::is_primitive(basic_type_mirror),
"just checking");
return java_lang_Class::primitive_type(basic_type_mirror);
}

// Narrowing of basic types. Used to create correct jvalues for
// boolean, byte, char and short return return values from interpreter
// which are returned as ints. Throws IllegalArgumentException.
Expand Down