On Wed, 29 Apr 2026 08:44:45 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:
> 
>   Make sure test is running with -RestrictContended

Thank you for the reviews everyone! 

We've had some offline discussion about not limiting the new test as much with 
`@requires`-properties. Right now running the test with ZGC won't provoke the 
original problem, and running without contended won't as well. The current 
`@requires`-properties make sure we run the test in a configuration that tests 
the originally affected code-path.

In the future we might not want to limit the test not to be run with ZGC. We 
should revisit the test when objects containing oops will be commonly flattened 
with ZGC.

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

PR Comment: https://git.openjdk.org/valhalla/pull/2371#issuecomment-4342641542

Reply via email to