On Thu, 11 Dec 2025 09:55:08 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... > > src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1481: > >> 1479: const bool full_header_in_current_region = cur_addr + >> oopDesc::header_size() <= end; >> 1480: >> 1481: if (EnableValhalla && !full_header_in_current_region) { > > We could make this criteria even more specific by checking something like the > following, but I'm not sure it's worth it since not copying the full header > in the same region is pretty rare. > > > static bool requires_valhalla_preservation(markWord mark) { > return EnableValhalla && (mark.is_larval_state() || > mark.is_null_free_array() || mark.is_flat_array() || mark.is_inline_type()); > } If I understand it correctly, the version you have right now is less conservative than `requires_valhalla_preservation` which may include false positives. I think I'm inclined to checking region containment, it also makes maintainability easier down the line (if we end up changing the markWord bits). ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1785#discussion_r2610484465
