On Fri, 12 Dec 2025 09:07:48 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 two additional > commits since the last revision: > > - Remove dev logging > - Convert COH check to assert in safe_to_read_header src/hotspot/share/gc/parallel/psParallelCompact.cpp line 2147: > 2145: } > 2146: > 2147: static markWord safe_mark_prototype(HeapWord* addr, size_t words) { Could this function be named `safe_mark_word_prototype` instead? src/hotspot/share/gc/parallel/psParallelCompact.cpp line 2279: > 2277: markWord mark = safe_mark_prototype(cur_addr, remaining_words); > 2278: > 2279: closure.do_addr(cur_addr, obj_size, mark); The fact that `do_addr` calls a function named `words_remaining()` and the current code calculates a `remaining_words` feels a bit messy and it's not immediately obvious that this is correct. Is there a reason why this can't be handled from withing the `do_addr` function? ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1785#discussion_r2613548668 PR Review Comment: https://git.openjdk.org/valhalla/pull/1785#discussion_r2613572830
