[lworld] RFR: 8311219: [lworld] VM option "InlineFieldMaxFlatSize" cannot work well

[Xiaohong Gong]

On Fri, 21 Jul 2023 02:39:53 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Currently all the non-static final fields with inline type can be flattened, even if the layout size of the inline type is beyond
>> the max flat size specified by `InlineFieldMaxFlatSize`. Please refer to the condition check [1] which decides whether a field
>> can be flattened or not.
>> 
>> Field flattening has two major side effects: atomicity and size. Fields with atomic access limitation or large size that exceeds
>> the specified threshold value cannot be flattened. And final fields are special that they are immutable after initialized. So the atomic check for them can be ignored. Hence, 1) for the atomicity free type like the primitive class, the final and non-final fields with such type can be flattened. And 2) for the normal value class that has atomic feature, only the final fields with such type can be flattened. And all kinds of the flattened fields should not exceed the specified max flat size. Please see more details from [1] [2].
>> 
>> The original condition [1] matches the atomicity check but not the flat size limitation. Promoting the flat size check before all other checks matches the flattening policy and can make the VM option `InlineFieldMaxFlatSize` work for final fields as well.
>> 
>> This patch also fixed the jtreg crashes involved after the field flattening condition is changed. Those tests fail with setting
>> `-XX:+InlineFieldMaxFlatSize=0` by default. The main issue is the non-flattened inline type field is not buffered which is expected to be. The root cause is when parsing `withfield`, the compiler checks whether the field is primitive class type while not its flattening status. Changing to check the flattening status instead can fix the crash.
>> 
>> [1] https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/classfile/fieldLayoutBuilder.cpp#L759
>> [2] https://mail.openjdk.org/pipermail/valhalla-dev/2023-June/011262.html
>> [3] https://mail.openjdk.org/pipermail/valhalla-dev/2023-July/011265.html
>
> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 661:
> 
>> 659:         }
>> 660:         if (!too_big_to_flatten &&
>> 661:             (!(too_atomic_to_flatten | too_volatile_to_flatten) || fieldinfo.access_flags().is_final())) {
> 
> Hi @XiaohongGong , specifications (https://openjdk.org/projects/valhalla/design-notes/state-of-valhalla/03-vm-model#flattenable-contexts) also talks about flattening final L descriptor fields of value classes.

Thanks for pointing out this! I got through of the specifications just now.

Final fields containing value objects, under both L and Q descriptors, can be routinely flattened. Mutable fields containing value objects under Q descriptors can be routinely flattened; mutable fields containing value objects under L descriptors must provide atomic loads and stores, which requires either atomic instructions or an indirection.

Per my understanding, this means:
1. The final fields can be flattened for all L/Q descriptors
2. The non-final fields can be flattened for Q descriptors
3. The non-final fields cannot be flattened for L descriptor value objects due to the atomic feature.


For value objects with many fields, the JVM may decide to fall back to buffering its value in the heap, which is the traditional indirected representation.

And the above means size of the flattened fields should not exceed some value. Right? 

Please correct me if any misunderstanding! Thanks!

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/888#discussion_r1270177471