On Thu, 3 Sep 2026 13:27:30 GMT, Frederic Parain <[email protected]> wrote:
>> Ioi Lam has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - @fparain review comments
>> - Changed inline_xxx to flat_xxx
>
> src/hotspot/share/cds/heapShared.cpp line 1784:
>
>> 1782: }
>> 1783:
>> 1784: // Recursively scan for any InlineKlass K that has least one non-null
>> inlined instance
>
> We've tried to standardize the terminology by using "flat" everywhere -> flat
> field, flat array, etc.
> Before that, we had too many variations: flat, flatten, flattened, inline,
> inlined, etc.
> We might have missed some spots, but this renaming is going against this
> effort.
I changed all terminology to use "flatten" instead of "inline".
> src/hotspot/share/oops/flatArrayKlass.cpp line 438:
>
>> 436: if (base_offset >= max_jint - 0x1000) {
>> 437: // TODO: the oopDesc::xxx_at() APIs are limited to int offsets,
>> but it's
>> 438: // possible for a value object be inlined at an offset higher
>> than 0x7ffffffff.
>
> `FlatArrayKlass::max_elements()` is supposed to prevent this from happening.
> Did you find a bug in it?
I added a new function `flatArrayOopDesc::value_offset_as_int()`.
> src/hotspot/share/runtime/fieldDescriptor.cpp line 132:
>
>> 130: // vk : InstanceKlass java/lang/Integer (we
>> are printing a field in an inlined Integer)
>> 131: // vk->payload_offset() : 8 (the payload starts at 8 bytes
>> above a regular Integer heap oop)
>> 132: // inline_offset : 16 (this inlined Integer starts at
>> offset 16 of obj
>
> Nit: missing closing parenthesis
Fixed.
> src/hotspot/share/runtime/fieldDescriptor.cpp line 132:
>
>> 130: // vk : InstanceKlass java/lang/Integer (we
>> are printing a field in an inlined Integer)
>> 131: // vk->payload_offset() : 8 (the payload starts at 8 bytes
>> above a regular Integer heap oop)
>> 132: // inline_offset : 16 (this inlined Integer starts at
>> offset 16 of obj
>
> The 16 bytes offset looks strange.
> Here's the layout of class Point with the default VM configuration (COH on):
>
> Layout of class Point@0x7f5140300790 extends java/lang/Object@0x7f514007c030
> Instance fields:
> @0 RESERVED 8/-
> @8 FLAT 4/4 "x" Ljava/lang/Integer; java/lang/Integer@0x7f514007c030
> NULL_FREE_NON_ATOMIC_FLAT
> @12 FLAT 4/4 "y" Ljava/lang/Integer; java/lang/Integer@0x7f514007c030
> NULL_FREE_NON_ATOMIC_FLAT
> @16 NULL_MARKER 1/1
> Static fields:
> @0 RESERVED 120/-
> @120 REGULAR 4/4 ".null_reset" Ljava/lang/Object;
> @124 REGULAR 4/4 ".acmp_maps" [I
> Instance size = 24 bytes
> First field offset = 8
> BUFFERED layout: 9/8
> NULL_FREE_NON_ATOMIC_FLAT layout: -/-
> NULL_FREE_ATOMIC_FLAT layout: 8/8
> NULLABLE_ATOMIC_FLAT layout: -/-
> NULLABLE_NON_ATOMIC_FLAT layout: 9/4
> Null marker offset = 16
> Non-oop acmp map <offset,size>: <8,8>
> oop acmp map:
Fixed. It should be 12.
> src/hotspot/share/runtime/fieldDescriptor.cpp line 176:
>
>> 174: st->print(" (fields 0x%08x) ", field_flags().as_uint());
>> 175: signature()->print_value_on(st);
>> 176: if (fc == nullptr) {
>
> Suggested change:
>
> st->print(" @%d ", fc == nullptr ? offset() : field_offset_in_obj(fc));
Fixed.
> test/hotspot/jtreg/runtime/PrintingTests/ValueObjectPrinting.java line 207:
>
>> 205: d = 1000001004;
>> 206: this.p2 = new Point(x2, y2);
>> 207: super();
>
> This is a value class, javac should automatically insert the implicit call to
> the super constructor at the end of this constructor (applies to the other
> value classes below).
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3928195865
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3928192064
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3928181469
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3928183482
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3928184231
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3928185102
