Skip to content

8371915: [lworld] LayoutKind enum should have helper methods #1745

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

Open
wants to merge 2 commits into
base: lworld
Choose a base branch
from
Open

8371915: [lworld] LayoutKind enum should have helper methods #1745

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
f10829c
Add helper methods to LayoutKind
fparain Nov 18, 2025
bb659fc
Fix ArrayProperties for NULLABLE_ATOMIC_FLAT
fparain Nov 20, 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
2 changes: 1 addition & 1 deletion src/hotspot/share/ci/ciArray.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ bool ciArray::is_atomic() {
}
if (oop->is_flatArray()) {
FlatArrayKlass* fak = FlatArrayKlass::cast(oop->klass());
if (fak->element_klass()->is_naturally_atomic() || fak->layout_kind() == LayoutKind::ATOMIC_FLAT || fak->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT) {
if (fak->element_klass()->is_naturally_atomic() || LayoutKindHelper::is_atomic_flat(fak->layout_kind())) {
return true;
}
}
Expand Down
18 changes: 18 additions & 0 deletions src/hotspot/share/oops/arrayKlass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,24 @@ oop ArrayKlass::component_mirror() const {
return java_lang_Class::component_mirror(java_mirror());
}

ArrayKlass::ArrayProperties ArrayKlass::array_properties_from_layout(LayoutKind lk) {
ArrayKlass::ArrayProperties props = ArrayKlass::ArrayProperties::DEFAULT;
switch(lk) {
case LayoutKind::ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NULL_RESTRICTED;
break;
case LayoutKind::NON_ATOMIC_FLAT:
props = (ArrayKlass::ArrayProperties)(ArrayKlass::ArrayProperties::NULL_RESTRICTED | ArrayKlass::ArrayProperties::NON_ATOMIC);
break;
case LayoutKind::NULLABLE_ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::DEFAULT;
break;
default:
ShouldNotReachHere();
}
return props;
}

Copy link
Contributor

@coleenp coleenp Nov 20, 2025

Choose a reason for hiding this comment

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

This is a good refactoring. nit: I think the coding style has the break indented with the props = lines.

// JVMTI support

jint ArrayKlass::jvmti_class_status() const {
Expand Down
5 changes: 4 additions & 1 deletion src/hotspot/share/oops/arrayKlass.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define SHARE_OOPS_ARRAYKLASS_HPP

#include "oops/klass.hpp"
#include "oops/layoutKind.hpp"

class fieldDescriptor;
class klassVtable;
Expand All @@ -38,7 +39,7 @@ class ArrayKlass: public Klass {

public:
enum ArrayProperties : uint32_t {
DEFAULT = 0,
DEFAULT = 0, // NULLABLE and ATOMIC
Copy link
Contributor

@DanHeidinga DanHeidinga Nov 20, 2025

Choose a reason for hiding this comment

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

Does DEFAULT also apply to arrays of oops? The name "DEFAULT" is slightly concerning if we're only talking about non-oop cases. I'd be tempted to call this NULLABLE_ATOMIC rather than DEFAULT

Copy link
Collaborator Author

@fparain fparain Nov 20, 2025

Choose a reason for hiding this comment

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

An array of oops (non-flat array) can be either DEFAULT or NULL_RESTRICTED.
DEFAULT is far from a perfect name, but it tries to reflect the fact that it is the semantic properties of arrays users get by default when creating an array, without using any special array creation API.

NULL_RESTRICTED = 1 << 0,
NON_ATOMIC = 1 << 1,
// FINAL = 1 << 2,
Expand All @@ -50,6 +51,8 @@ class ArrayKlass: public Klass {
static bool is_null_restricted(ArrayProperties props) { return (props & NULL_RESTRICTED) != 0; }
static bool is_non_atomic(ArrayProperties props) { return (props & NON_ATOMIC) != 0; }

static ArrayProperties array_properties_from_layout(LayoutKind lk);

private:
// If you add a new field that points to any metaspace object, you
// must add this field to ArrayKlass::metaspace_pointers_do().
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/oops/flatArrayKlass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
FlatArrayKlass::FlatArrayKlass(Klass* element_klass, Symbol* name, ArrayProperties props, LayoutKind lk) :
ObjArrayKlass(1, element_klass, name, Kind, props, markWord::flat_array_prototype(lk)) {
assert(element_klass->is_inline_klass(), "Expected Inline");
assert(lk == LayoutKind::NON_ATOMIC_FLAT || lk == LayoutKind::ATOMIC_FLAT || lk == LayoutKind::NULLABLE_ATOMIC_FLAT, "Must be a flat layout");
assert(LayoutKindHelper::is_flat(lk), "Must be a flat layout");

set_element_klass(InlineKlass::cast(element_klass));
set_class_loader_data(element_klass->class_loader_data());
Expand Down Expand Up @@ -151,7 +151,7 @@ jint FlatArrayKlass::array_layout_helper(InlineKlass* vk, LayoutKind lk) {
BasicType etype = T_FLAT_ELEMENT;
int esize = log2i_exact(round_up_power_of_2(vk->layout_size_in_bytes(lk)));
int hsize = arrayOopDesc::base_offset_in_bytes(etype);
bool null_free = lk != LayoutKind::NULLABLE_ATOMIC_FLAT;
bool null_free = !LayoutKindHelper::is_nullable_flat(lk);
int lh = Klass::array_layout_helper(_lh_array_tag_flat_value, null_free, hsize, etype, esize);

assert(lh < (int)_lh_neutral_value, "must look like an array layout");
Expand Down Expand Up @@ -286,7 +286,7 @@ void FlatArrayKlass::copy_array(arrayOop s, int src_pos,
flatArrayHandle hd(THREAD, da);
flatArrayHandle hs(THREAD, sa);
// source and destination layouts mismatch, simpler solution is to copy through an intermediate buffer (heap instance)
bool need_null_check = fsk->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT && fdk->layout_kind() != LayoutKind::NULLABLE_ATOMIC_FLAT;
bool need_null_check = LayoutKindHelper::is_nullable_flat(fsk->layout_kind()) && !LayoutKindHelper::is_nullable_flat(fdk->layout_kind());
oop buffer = vk->allocate_instance(CHECK);
address dst = (address) hd->value_at_addr(dst_pos, fdk->layout_helper());
address src = (address) hs->value_at_addr(src_pos, fsk->layout_helper());
Expand Down
14 changes: 14 additions & 0 deletions src/hotspot/share/oops/layoutKind.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define SHARE_OOPS_LAYOUTKIND_HPP

#include "utilities/globalDefinitions.hpp"
#include "memory/allStatic.hpp"

// LayoutKind is an enum used to indicate which layout has been used for a given value field.
// Each layout has its own properties and its own access protocol that is detailed below.
Expand Down Expand Up @@ -81,4 +82,17 @@ enum class LayoutKind : uint32_t {
UNKNOWN = 5 // used for uninitialized fields of type LayoutKind
};

class LayoutKindHelper : AllStatic {
Copy link
Contributor

@coleenp coleenp Nov 20, 2025

Choose a reason for hiding this comment

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

I wonder if this would be nicer if LayoutKind was a class and these helpers were in that class and encapsulate the whole thing in LayoutKind, so the accessors could be lk->is_flat(), etc? less characters. The enum inside LayoutKind could be referenced as LayoutKind::Kind::REFERENCE if direct references to the value is needed. Other enum class types in the VM are used that way (like AccessFlags). Then also it wouldn't need a helper.

public:
static bool is_flat(LayoutKind lk) {
return lk == LayoutKind::NON_ATOMIC_FLAT || lk == LayoutKind::ATOMIC_FLAT || lk == LayoutKind::NULLABLE_ATOMIC_FLAT;
}
static bool is_atomic_flat(LayoutKind lk) {
return lk == LayoutKind::ATOMIC_FLAT || lk == LayoutKind::NULLABLE_ATOMIC_FLAT;
}
static bool is_nullable_flat(LayoutKind lk) {
return lk == LayoutKind::NULLABLE_ATOMIC_FLAT;
}
};

#endif // SHARE_OOPS_LAYOUTKIND_HPP
12 changes: 6 additions & 6 deletions src/hotspot/share/opto/library_call.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2887,8 +2887,8 @@ bool LibraryCallKit::inline_unsafe_flat_access(bool is_store, AccessKind kind) {
} else {
if (UseArrayFlattening) {
// Flat array must have an exact type
bool is_null_free = layout != LayoutKind::NULLABLE_ATOMIC_FLAT;
bool is_atomic = layout != LayoutKind::NON_ATOMIC_FLAT;
bool is_null_free = !LayoutKindHelper::is_nullable_flat(layout);
bool is_atomic = LayoutKindHelper::is_atomic_flat(layout);
Node* new_base = cast_to_flat_array(base, value_klass, is_null_free, !is_null_free, is_atomic);
replace_in_map(base, new_base);
base = new_base;
Expand Down Expand Up @@ -2924,8 +2924,8 @@ bool LibraryCallKit::inline_unsafe_flat_access(bool is_store, AccessKind kind) {
const TypePtr* ptr_type = (decorators & C2_MISMATCHED) != 0 ? TypeRawPtr::BOTTOM : _gvn.type(ptr)->is_ptr();
access_store_at(base, ptr, ptr_type, value, value_type, T_OBJECT, decorators);
} else {
bool atomic = layout != LayoutKind::NON_ATOMIC_FLAT;
bool null_free = layout != LayoutKind::NULLABLE_ATOMIC_FLAT;
bool atomic = LayoutKindHelper::is_atomic_flat(layout);
bool null_free = !LayoutKindHelper::is_nullable_flat(layout);
value->as_InlineType()->store_flat(this, base, ptr, atomic, immutable_memory, null_free, decorators);
}

Expand All @@ -2938,8 +2938,8 @@ bool LibraryCallKit::inline_unsafe_flat_access(bool is_store, AccessKind kind) {
Node* oop = access_load_at(base, ptr, ptr_type, Type::get_const_type(value_klass), T_OBJECT, decorators);
result = InlineTypeNode::make_from_oop(this, oop, value_klass);
} else {
bool atomic = layout != LayoutKind::NON_ATOMIC_FLAT;
bool null_free = layout != LayoutKind::NULLABLE_ATOMIC_FLAT;
bool atomic = LayoutKindHelper::is_atomic_flat(layout);
bool null_free = !LayoutKindHelper::is_nullable_flat(layout);
result = InlineTypeNode::make_from_flat(this, value_klass, base, ptr, atomic, immutable_memory, null_free, decorators);
}

Expand Down
15 changes: 1 addition & 14 deletions src/hotspot/share/opto/runtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,20 +378,7 @@ JRT_BLOCK_ENTRY(void, OptoRuntime::new_array_C(Klass* array_type, int len, oopDe
Handle holder(current, array_type->klass_holder()); // keep the array klass alive
FlatArrayKlass* fak = FlatArrayKlass::cast(array_type);
InlineKlass* vk = fak->element_klass();
ArrayKlass::ArrayProperties props = ArrayKlass::ArrayProperties::DEFAULT;
switch(fak->layout_kind()) {
case LayoutKind::ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NULL_RESTRICTED;
break;
case LayoutKind::NON_ATOMIC_FLAT:
props = (ArrayKlass::ArrayProperties)(ArrayKlass::ArrayProperties::NULL_RESTRICTED | ArrayKlass::ArrayProperties::NON_ATOMIC);
break;
case LayoutKind::NULLABLE_ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NON_ATOMIC;
break;
default:
ShouldNotReachHere();
}
ArrayKlass::ArrayProperties props = ArrayKlass::array_properties_from_layout(fak->layout_kind());
result = oopFactory::new_flatArray(vk, len, props, fak->layout_kind(), THREAD);
if (array_type->is_null_free_array_klass() && !h_init_val.is_null()) {
// Null-free arrays need to be initialized
Expand Down
19 changes: 2 additions & 17 deletions src/hotspot/share/prims/jvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,20 +445,7 @@ JVM_ENTRY(jarray, JVM_CopyOfSpecialArray(JNIEnv *env, jarray orig, jint from, ji
if (org->is_flatArray()) {
FlatArrayKlass* fak = FlatArrayKlass::cast(org->klass());
LayoutKind lk = fak->layout_kind();
ArrayKlass::ArrayProperties props = ArrayKlass::ArrayProperties::DEFAULT;
switch(lk) {
case LayoutKind::ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NULL_RESTRICTED;
break;
case LayoutKind::NON_ATOMIC_FLAT:
props = (ArrayKlass::ArrayProperties)(ArrayKlass::ArrayProperties::NULL_RESTRICTED | ArrayKlass::ArrayProperties::NON_ATOMIC);
break;
case LayoutKind::NULLABLE_ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NON_ATOMIC;
break;
default:
ShouldNotReachHere();
}
ArrayKlass::ArrayProperties props = ArrayKlass::array_properties_from_layout(lk);
array = oopFactory::new_flatArray(vk, len, props, lk, CHECK_NULL);
arrayHandle ah(THREAD, (arrayOop)array);
int end = to < oh()->length() ? to : oh()->length();
Expand Down Expand Up @@ -556,9 +543,7 @@ JVM_ENTRY(jboolean, JVM_IsAtomicArray(JNIEnv *env, jobject obj))
if (oop->is_refArray()) return true;
if (oop->is_flatArray()) {
FlatArrayKlass* fak = FlatArrayKlass::cast(oop->klass());
if (fak->layout_kind() == LayoutKind::ATOMIC_FLAT || fak->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT) {
return true;
}
if (LayoutKindHelper::is_atomic_flat(fak->layout_kind())) return true;
if (fak->element_klass()->is_naturally_atomic()) return true;
}
return false;
Expand Down
10 changes: 4 additions & 6 deletions src/hotspot/share/prims/jvmtiTagMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1398,8 +1398,7 @@ void IterateThroughHeapObjectClosure::visit_flat_fields(const JvmtiHeapwalkObjec
field_offset += obj.offset() - obj.inline_klass()->payload_offset();
}
// check for possible nulls
bool can_be_null = field->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT;
if (can_be_null) {
if (LayoutKindHelper::is_nullable_flat(field->layout_kind())) {
address payload = cast_from_oop<address>(obj.obj()) + field_offset;
if (field->inline_klass()->is_payload_marked_as_null(payload)) {
continue;
Expand All @@ -1421,7 +1420,7 @@ void IterateThroughHeapObjectClosure::visit_flat_array_elements(const JvmtiHeapw
flatArrayOop array = flatArrayOop(obj.obj());
FlatArrayKlass* faklass = FlatArrayKlass::cast(array->klass());
InlineKlass* vk = InlineKlass::cast(faklass->element_klass());
bool need_null_check = faklass->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT;
bool need_null_check = LayoutKindHelper::is_nullable_flat(faklass->layout_kind());

for (int index = 0; index < array->length(); index++) {
address addr = (address)array->value_at_addr(index, faklass->layout_helper());
Expand Down Expand Up @@ -2876,7 +2875,7 @@ inline bool VM_HeapWalkOperation::iterate_over_flat_array(const JvmtiHeapwalkObj
flatArrayOop array = flatArrayOop(o.obj());
FlatArrayKlass* faklass = FlatArrayKlass::cast(array->klass());
InlineKlass* vk = InlineKlass::cast(faklass->element_klass());
bool need_null_check = faklass->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT;
bool need_null_check = LayoutKindHelper::is_nullable_flat(faklass->layout_kind());

// array reference to its class
oop mirror = faklass->java_mirror();
Expand Down Expand Up @@ -3100,8 +3099,7 @@ inline bool VM_HeapWalkOperation::iterate_over_object(const JvmtiHeapwalkObject&
if (!is_primitive_field_type(type)) {
if (field->is_flat()) {
// check for possible nulls
bool can_be_null = field->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT;
if (can_be_null) {
if (LayoutKindHelper::is_nullable_flat(field->layout_kind())) {
address payload = cast_from_oop<address>(o.obj()) + field_offset;
if (field->inline_klass()->is_payload_marked_as_null(payload)) {
continue;
Expand Down
15 changes: 1 addition & 14 deletions src/hotspot/share/prims/unsafe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,20 +433,7 @@ UNSAFE_ENTRY(jarray, Unsafe_NewSpecialArray(JNIEnv *env, jobject unsafe, jclass
if (!UseArrayFlattening || !vk->is_layout_supported(lk)) {
THROW_MSG_NULL(vmSymbols::java_lang_UnsupportedOperationException(), "Layout not supported");
}
ArrayKlass::ArrayProperties props = ArrayKlass::ArrayProperties::DEFAULT;
switch(lk) {
case LayoutKind::ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NULL_RESTRICTED;
break;
case LayoutKind::NON_ATOMIC_FLAT:
props = (ArrayKlass::ArrayProperties)(ArrayKlass::ArrayProperties::NULL_RESTRICTED | ArrayKlass::ArrayProperties::NON_ATOMIC);
break;
case LayoutKind::NULLABLE_ATOMIC_FLAT:
props = ArrayKlass::ArrayProperties::NON_ATOMIC;
break;
default:
ShouldNotReachHere();
}
ArrayKlass::ArrayProperties props = ArrayKlass::array_properties_from_layout(lk);
oop array = oopFactory::new_flatArray(vk, len, props, lk, CHECK_NULL);
return (jarray) JNIHandles::make_local(THREAD, array);
} UNSAFE_END
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/services/heapDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ class DumperClassCacheTableEntry : public CHeapObj<mtServiceability> {
bool is_flat() const { return _inline_klass != nullptr; }
InlineKlass* inline_klass() const { return _inline_klass; }
LayoutKind layout_kind() const { return _layout_kind; }
bool is_flat_nullable() const { return _layout_kind == LayoutKind::NULLABLE_ATOMIC_FLAT; }
bool is_flat_nullable() const { return LayoutKindHelper::is_nullable_flat(_layout_kind); }
};

private:
Expand Down Expand Up @@ -1481,7 +1481,7 @@ void DumperSupport::dump_object_array(AbstractDumpWriter* writer, objArrayOop ar
FlatArrayKlass* faklass = FlatArrayKlass::cast(farray->klass());

InlineKlass* vk = faklass->element_klass();
bool need_null_check = faklass->layout_kind() == LayoutKind::NULLABLE_ATOMIC_FLAT;
bool need_null_check = LayoutKindHelper::is_nullable_flat(faklass->layout_kind());

for (int index = 0; index < length; index++) {
address addr = (address)farray->value_at_addr(index, faklass->layout_helper());
Expand Down