On Thu, 3 Sep 2026 09:21:11 GMT, Stefan Karlsson <[email protected]> wrote:
> JEP 401 delivered the preview feature of Value Objects. > > During the lifetime of the Valhalla project there were many ideas and names, > and at one point the value types were called inline types. The HotSpot code > was changed to use the name "inline types" instead of "value types" (and > inline klass instead of value klass) > [JDK-8249257](https://bugs.openjdk.org/browse/JDK-8249257). > > Later the concept and name "inline type" were dropped, but HotSpot retained > the name. This causes confusion when working with the code. Some reasons: > > 1) There's no obvious connection to what's described in JEP 401 or the Java > code. > > 2) It is easy to incorrectly assume that these are types that have been > flattened (inlined) into other objects. > > 3) The name clash with C++'s concept of inlining > > My proposal is that we revert back to talking about "values" instead of > "inlines", and that we do this before JDK 28 ships, so that we don't have a > release where we use the "inline" terminology. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Thanks Stefan for taking this on! Some initial comments on places were we missed the `inline`->`value` rename. Some of them may be better renamed as `flat` to match the flat array precedent. I haven't looked at everything yet but audited some of the cds/, classfile/, and oops/ code as a starting point. We should also consider what other PRs are in flight that may conflict with this. I know Vladimir has his large AOT code caching PR (once the JEP makes its way through the process) that it might be worth coordinating together about which goes first. Possibly other large PRs in the works as well? src/hotspot/share/asm/macroAssembler_common.cpp line 82: > 80: > 81: #ifdef COMPILER2 > 82: int MacroAssembler::unpack_inline_args(Compile* C, bool receiver_only) { Suggestion: int MacroAssembler::unpack_value_args(Compile* C, bool receiver_only) { This should probably get the inline->value treatment as well src/hotspot/share/asm/macroAssembler_common.cpp line 199: > 197: } > 198: > 199: bool MacroAssembler::shuffle_inline_args_spill(bool is_packing, const > GrowableArray<SigEntry>* sig, int sig_index, Suggestion: bool MacroAssembler::shuffle_value_args_spill(bool is_packing, const GrowableArray<SigEntry>* sig, int sig_index, Looks like some "inline" got missed in the conversion. src/hotspot/share/asm/macroAssembler_common.hpp line 60: > 58: VMRegPair* from, int from_count, int& > from_index, VMReg to, > 59: RegState reg_state[], Register val_array); > 60: int extend_stack_for_inline_args(int args_on_stack); shuffle_inline_args_spill -> shuffle_value_args_spill unpack_inline_helper -> unpack_value_helper pack_inline_helper -> pack_value_helper extend_stack_for_inline_args -> extend_stack_for_value_args src/hotspot/share/cds/heapShared.cpp line 1780: > 1778: // Found a non-null flattened instance of vk. Let's record vk. > 1779: add_value_class(_subgraph_info, vk); > 1780: if (vk->has_inlined_fields()) { Suggestion: if (vk->has_value_fields()) { Or may `has_flat_fields()`? src/hotspot/share/classfile/classFileParser.cpp line 6592: > 6590: > 6591: // Set the Klass for the field's index > 6592: _inline_layout_info_array->adr_at(field_index)->set_klass(vk); Should `_inline_layout_info_array` be renamed to `_value_layout_info_array` for consistency with the new naming convention? src/hotspot/share/classfile/classFileParser.hpp line 257: > 255: void set_klass(InstanceKlass* instance); > 256: > 257: void set_inline_layout_info_klass(int field_index, ValueKlass* vk, > TRAPS); Suggestion: void set_value_layout_info_klass(int field_index, ValueKlass* vk, TRAPS); src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 73: > 71: } > 72: > 73: InlineLayoutInfo* inline_field_info = > inline_layout_info_array->adr_at(field_info.index()); Suggestion: ValueLayoutInfo* value_field_info = value_layout_info_array->adr_at(field_info.index()); src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 74: > 72: > 73: InlineLayoutInfo* inline_field_info = > inline_layout_info_array->adr_at(field_info.index()); > 74: ValueKlass* vk = inline_field_info->klass(); Suggestion: ValueKlass* vk = value_field_info->klass(); src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 111: > 109: if (lk == LayoutKind::REFERENCE) return lk; > 110: assert(LayoutKindHelper::is_flat((lk)), "Must be"); > 111: InlineLayoutInfo* inline_field_info = > inline_layout_info_array->adr_at(field_info.index()); Suggestion: ValueLayoutInfo* inline_field_info = value_layout_info_array->adr_at(field_info.index()); There's some renaming that was missed in this file. To be consistent, we should use inline->value in all these places or, if we're talking about fields that have been flattened and want a distinction, we should use flat rather than inline src/hotspot/share/classfile/fieldLayoutBuilder.hpp line 281: > 279: GrowableArray<FieldInfo>* _field_info; > 280: FieldLayoutInfo* _info; > 281: Array<InlineLayoutInfo>* _inline_layout_info_array; Suggestion: Array<ValueLayoutInfo>* _value_layout_info_array; src/hotspot/share/classfile/fieldLayoutBuilder.hpp line 307: > 305: bool _has_nonstatic_fields; > 306: bool _has_inlineable_fields; > 307: bool _has_inlined_fields; We need to check these two to see what the proper inline->value mapping is. Or maybe they are still correctly referring to inlined values.... needs some eyes to confirm src/hotspot/share/classfile/verificationType.hpp line 71: > 69: // Enum for the _data field > 70: enum : uint { > 71: // Bottom three bits determine if the type is a reference, value > type, I think this comment is a left over from earlier models. The verifier doesn't track value types separately. Will open a separate cleanup bug to check the comments in verificationType to remove Inline/Value where it doesn't apply src/hotspot/share/classfile/verifier.cpp line 2737: > 2735: else if (opcode == Bytecodes::_invokespecial > 2736: && !is_same_or_direct_interface(current_class(), > current_type(), ref_class_type) > 2737: && > !ref_class_type.equals(VerificationType::reference_type(current_class()->super()->name()))) > { // super() can never be a value_type. The rename matches but I don't think this comment is correct any more. Will add it to the cleanup bug mentioned above src/hotspot/share/code/nmethod.hpp line 212: > 210: uint16_t _entry_offset; // entry point with class check > 211: uint16_t _verified_entry_offset; // entry point without class check > 212: uint16_t _inline_entry_offset; // value type entry point > (unpack all value type args) with class check Suggestion: uint16_t _value_entry_offset; // value type entry point (unpack all value type args) with class check src/hotspot/share/code/nmethod.hpp line 213: > 211: uint16_t _verified_entry_offset; // entry point without class check > 212: uint16_t _inline_entry_offset; // value type entry point > (unpack all value type args) with class check > 213: uint16_t _verified_inline_entry_offset; // value type entry point > (unpack all value type args) without class check Suggestion: uint16_t _verified_value_entry_offset; // value type entry point (unpack all value type args) without class check src/hotspot/share/code/nmethod.hpp line 214: > 212: uint16_t _inline_entry_offset; // value type entry point > (unpack all value type args) with class check > 213: uint16_t _verified_inline_entry_offset; // value type entry point > (unpack all value type args) without class check > 214: uint16_t _verified_inline_ro_entry_offset; // value type entry point > (unpack receiver only) without class check Suggestion: uint16_t _verified_value_ro_entry_offset; // value type entry point (unpack receiver only) without class check src/hotspot/share/code/nmethod.hpp line 688: > 686: address inline_entry_point() const { return code_begin() > + _inline_entry_offset; } // value type entry point (unpack all > value type args) > 687: address verified_inline_entry_point() const { return code_begin() > + _verified_inline_entry_offset; } // value type entry point (unpack all > value type args) without class check > 688: address verified_inline_ro_entry_point() const { return code_begin() > + _verified_inline_ro_entry_offset; } // value type entry point (only unpack > receiver) without class check Suggestion: address value_entry_point() const { return code_begin() + _value_entry_offset; } // value type entry point (unpack all value type args) address verified_value_entry_point() const { return code_begin() + _verified_value_entry_offset; } // value type entry point (unpack all value type args) without class check address verified_value_ro_entry_point() const { return code_begin() + _verified_value_ro_entry_offset; } // value type entry point (only unpack receiver) without class check src/hotspot/share/oops/instanceKlass.hpp line 329: > 327: Array<FieldStatus>* _fields_status; > 328: > 329: Array<InlineLayoutInfo>* _inline_layout_info_array; Suggestion: Array<ValueLayoutInfo>* _value_layout_info_array; src/hotspot/share/oops/instanceKlass.hpp line 995: > 993: static ByteSize init_thread_offset() { return > byte_offset_of(InstanceKlass, _init_thread); } > 994: > 995: static ByteSize inline_layout_info_array_offset() { return > byte_offset_of(InstanceKlass, _inline_layout_info_array); } Suggestion: static ByteSize value_layout_info_array_offset() { return byte_offset_of(InstanceKlass, _value_layout_info_array); } src/hotspot/share/oops/instanceKlass.hpp line 1094: > 1092: assert(_inline_layout_info_array != nullptr, "Array not created"); > 1093: return _inline_layout_info_array->at(index); > 1094: } inline -> value needs to be applied here as well src/hotspot/share/oops/instanceKlass.hpp line 1096: > 1094: } > 1095: > 1096: InlineLayoutInfo* inline_layout_info_adr(int index) { Suggestion: ValueLayoutInfo* value_layout_info_adr(int index) { _inline_layout_info_array -> _value_layout_info_array src/hotspot/share/oops/instanceKlass.inline.hpp line 82: > 80: assert(has_inlined_fields() || has_null_restricted_static_fields(), > "Sanity checking"); > 81: assert(idx < java_fields_count(), "IOOB"); > 82: ValueKlass* k = inline_layout_info(idx).klass(); Suggestion: ValueKlass* k = value_layout_info(idx).klass(); src/hotspot/share/oops/instanceKlassFlags.hpp line 56: > 54: flag(has_miranda_methods , 1 << 12) /* True if this > class has miranda methods in it's vtable */ \ > 55: flag(has_final_method , 1 << 13) /* True if klass > has final method */ \ > 56: flag(has_inlined_fields , 1 << 14) /* has inlined > fields and related embedded section is not empty */ \ This one should be renamed as well but not not entirely sure if it should be "value" or "flattened". value to be consistent with the inline->value mapping, flattened to be consistent with flat array naming. There's a few places that need to be consistently modified still related to this decision ------------- PR Review: https://git.openjdk.org/jdk/pull/32673#pullrequestreview-5102291942 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925160644 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925171353 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925187616 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925211293 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924859808 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924863409 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924994418 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924996129 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925001622 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924984018 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924977088 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924903557 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924922513 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924940346 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924942360 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924943989 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3924950142 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925054894 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925062915 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925071776 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925068848 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925075203 PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3925089207
