On Tue, 28 Apr 2026 13:57:13 GMT, Joel Sikström <[email protected]> wrote:

>> Hello,
>> 
>> Embdedded flat contended fields might not be included in the class' OopMap 
>> if there are no other oop fields in the holder class.
>> 
>> Flat fields are handled separately in fieldLayoutBuilder.cpp, resulting in 
>> the current `_oop_count` not being updated if a flat field contains oops. 
>> This is OK, since the oop fields are handled at a later point in time. 
>> However, for contended fields, which are part of the contended group, 
>> embedded oop fields are only tracked if: `cg->oop_count() > 0`, which is not 
>> guaranteed to be true.
>> 
>> I've written a test to see if the oop field is included in the class' OopMap 
>> and also changed the `_oop_count` field `_has_embedded_oops`, and changed 
>> that to a boolean property, which is how we've used it so far.
>> 
>> Since embedded fields in flat paylods are not tracked in `cg->oop_fields()`, 
>> we can't do the assert that is currently there, so I just removed that.
>> 
>> Testing:
>> * New test fails without the fixes in fieldLayoutBuilder and passes with 
>> fixes
>> * Running Oracle's tier1-3
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Joel Sikström has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Test enhancement

test/hotspot/jtreg/runtime/valhalla/inlinetypes/ContendedFlatFieldOopMap.java 
line 39:

> 37:  * @enablePreview
> 38:  * @run driver jdk.test.lib.helpers.ClassFileInstaller 
> jdk.test.whitebox.WhiteBox
> 39:  * @run main/othervm -XX:-RestrictContended

Nit: it might be good to guard against this being set:
`@requires vm.opt.RestrictContended == null`
Or maybe even:
`@requires vm.opt.RestrictContended == null | vm.opt.RestrictContended == 
"false"`

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2371#discussion_r3159571287

Reply via email to