On Mon, 15 Dec 2025 10:20:16 GMT, Joel Sikström <[email protected]> wrote:

>> Hello,
>> 
>> The Valhalla-specific bits in the markWord need to be preserved when 
>> compacting objects in Parallel. This can easily be achieved in one of two 
>> ways:
>> 1) Use the markWord from the object's Klass, which contains all relevant 
>> bits. Notably, this requires access to the Klass pointer.
>> 2) Use the "PreservedMark" mechanism, preserving the markWord containing all 
>> relevant bits before compacting and restoring the markWord after comapcting.
>> 
>> In mainline, we only ever read the markWord from the object's Klass when 
>> CompactObjectHeaders is enabled. This is perfectly fine, since 
>> CompactObjectHeaders stores the Klass pointer in the markWord and Parallel 
>> will always copy **at least** one byte of the object from the current 
>> region. All other bytes of the object may be copied in another region, in 
>> which case it is not safe to read it concurrently while another thread might 
>> compact those bytes.
>> 
>> Now in Valhalla we also need to access the Klass pointer to get the 
>> markWord, even though we might not be using CompactObjectHeaders, which 
>> might not always be safe to do. To get around this, 
>> [JDK-8368875](https://bugs.openjdk.org/browse/JDK-8368875) opted to always 
>> use the PreservedMark mechanism instead, by changing the criteria for a 
>> markWord being preserved. Two impacts of this change stand out:
>> 1) We always preserve the markWord, even though we could be able to read the 
>> Klass pointer if the full object header was copied.
>> 2) Since the change was made to oopDesc::must_be_preserved(), which is not 
>> Parallel-specific, all GCs will now preserve the markWord as well for the 
>> same criteria, which is not needed.
>> 
>> I propose we change the full reliance on the PreservedMark mechanism to a 
>> specific solution for Parallel, which preserves the markWord only when we 
>> are not guaranteed to be able to access the Klass pointer, and uses the 
>> Klass pointer when possible. This way we preserve markWords only when 
>> absolutely necessary, and other GCs also don't have to take the hit of 
>> preserving markWords unnecessarily.
>> 
>> After this change, it has become relatively uncommon to see a markWord 
>> preserved for the new "Valhalla reason." This preservation now requires an 
>> object to cross a region boundary and to only have a single word in the 
>> current region (disregarding the use of CompactObjectHeaders). I've 
>> attempted to trigger this scenario by setting a small maximum heap size 
>> (-Xmx60M) in some tests, which sometimes results in a markWord being 
>> preserved. However, with a lar...
>
> Joel Sikström has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Comment for safe check in safe_to_read_header

Thank you everyone for the reviews and help with figuring this out. I've rerun 
ParallelGC specific testing on the tip of lworld which looks good.

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

PR Comment: https://git.openjdk.org/valhalla/pull/1785#issuecomment-3657068342

Reply via email to