On Tue, 28 Apr 2026 11:40:43 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: > > Remove _has_embedded_oops The code itself looks good, nice catch! Left some small comments about the test. test/hotspot/jtreg/runtime/valhalla/inlinetypes/ContendedFlatFieldOopMap.java line 32: > 30: * @test ContendedFlatFieldOopMap > 31: * @library /test/lib > 32: * @requires vm.flagless This will ignore other GCs than the default (likely G1). Is that intentional? test/hotspot/jtreg/runtime/valhalla/inlinetypes/ContendedFlatFieldOopMap.java line 66: > 64: Asserts.assertNotEquals(ref.get(), null); > 65: > 66: System.gc(); Is there any reason we prefer `System.gc` over `WhiteBox`' GC? ------------- PR Review: https://git.openjdk.org/valhalla/pull/2371#pullrequestreview-4188555818 PR Review Comment: https://git.openjdk.org/valhalla/pull/2371#discussion_r3153950848 PR Review Comment: https://git.openjdk.org/valhalla/pull/2371#discussion_r3153943581
