On Mon, 16 Feb 2026 14:51:52 GMT, Joel Sikström <[email protected]> wrote:

> Hello,
> 
> We should consider moving the enum ArrayProperties from ArrayKlass to its own 
> class, ArrayProperties. In addition to making the code easier to read and 
> understand, this allows us to have explicit setters/getters, replacing the 
> bit-fiddling expressions that are used in many places. The 
> ArrayProperties-specific methods in ArrayKlass have been moved to be methods 
> in the new ArrayProperties class instead.
> 
> Perhaps the most controversial change in this PR is the removal of 
> `ArrayKlass::ArrayProperties::DEFAULT` in favor of using a default 
> constructor for ArrayProperties. The semantics are still the same, i.e., 
> asking `.is_null_restricted()` or `.is_non_atomic()` will be false for the 
> default constructed property. With this I've also removed the unused fields 
> from ArrayProperties (DUMMY and comments).
> 
> I did consider using define macros to generate enum+getters+setters, but I 
> opted for the stamped-out version instead.
> 
> Testing:
> * Running through tier1-2

src/hotspot/share/oops/objArrayKlass.cpp line 425:

> 423:     if (next_refined_array_klass() == nullptr) {
> 424:       ObjArrayKlass* first = this;
> 425:       if (!is_refArray_klass() && !is_flatArray_klass() && 
> (props.is_null_restricted() || props.is_non_atomic())) {

The check for this was `props != ArrayKlass::ArrayProperties::DEFAULT` before 
this, which included the INVALID state. I ran this through tier1-2 with the 
assert `assert(!props.is_invalid(), "surely");`, which did not hit.

I'd appreciate feedback on if we still want to check for the invalid property 
here!

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2114#discussion_r2812812833

Reply via email to