On Fri, 9 Oct 2026 11:22:11 GMT, Stefan Karlsson <[email protected]> wrote:
> Today LayoutKind serves multiple purposes in the code:
>
> 1) Describe the layout of a value field [REFERENCE, *_FLAT]
> 2) Describe the layout of a field, flat layout not applicable or flat layout
> [UNKNOWN, *_FLAT]
> 3) Describe the layout of a value payload [BUFFERED, *_FLAT]
> 4) Describe uninitialized values for (1-3) [UNKNOWN]
>
> A surprising (to me) amount of code lands in (2).
>
> This leads to us having to be careful around the values that are invalid to
> use at the various locations in the code.
>
> I want to make it so that it is easier to know that you are passing around
> valid values and that we catch early if we start to use uninitialized values.
>
> So, I propose that we convert `LayoutKind` to only hold the flat layouts and
> then we add a couple of wrapper classes that support the optionality of
> holding a flat layout.
>
> Code that is guaranteed to operate on flat layouts will be using the
> `FlatLayout` class.
>
> Code that may operate on a flat layout will be using the `OptionalFlatLayout`
> class, which can be queried to see if it holds a `FlatLayout` or not.
>
> `OptionalFlatLayout` adds an extra `_is_flat` boolean that is used to
> discriminate if the `FlatLayout` is active or not. Often code already had an
> `_is_flat` field, and for those the existing flatness field has been
> superseded by the one in `OptionalFlatLayout`.
>
> Some classes already have their own, other way to decide if we are dealing
> with a flat layout. For those I've introduced a scaled-down version, called
> `FlatLayoutSlot`, where the existence of a `FlatLayout` is determined by the
> surrounding code's "is flat" discriminator.
>
> Code that is using `OptionalFlatLayout` fields can create it to hold a flat
> layout, create it to represent a non-flat situation, and create it marked as
> "uninitialized". It might be that the code doesn't yet know how to initialize
> it, or the code might only use it under certain conditions. To make sure that
> we don't try to query uninitialized OptionalFlatLayouts, asserts have been
> added to all layout queries.
>
> Other things to look for in the patch:
>
> * The implementation and construction of `ArrayDescription` has been
> simplified.
>
> * I've decoupled the LayoutKind enumeration values from the "layout value"
> passed around via Unsafe (and MethodHandle). See FlatLayoutEncoding.
>
> * Serviceability code has some restructuring around the construction of
> objects to make sure that they can be initialized properly in the initializer
> list.
>
> * `JvmtiFlatTagMapKey` has been renamed to `JvmtiValueTagMapKey...
Short review.
OK:
- /misc/Unsafe.java
- /runtime/fieldDescriptor.cpp
- /runtime/fieldDescriptor.hpp
OK with comment:
- /services/heapDumper.cpp
src/hotspot/share/services/heapDumper.cpp line 851:
> 849:
> 850: template<typename FieldStreamType>
> 851: static ValueFieldInfo* calc_value_field_info(const FieldStreamType&
> field) {
Is this `calc_` prefix that informative? `value_field_info` seems fine, as well
as `value_klass`. Or, if you're like me, you'll name them `$PREFIX_of`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/33283#pullrequestreview-5488220060
PR Review Comment: https://git.openjdk.org/jdk/pull/33283#discussion_r4246031041
