RFR: 8354929: Update collection stats while holding page allocator lock [v2]

Axel Boldt-Christmas aboldtch at openjdk.org
Tue Apr 22 06:18:42 UTC 2025


On Thu, 17 Apr 2025 18:15:21 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Please review this change to restructure some code in the mark start pause to do updates while holding the lock.
>> 
>> **Summary**
>> We currently update the collection high and low used values during the mark start pause without taking the page allocator lock. This is fine since it is read atomically, but consistency verification in this code requires the lock to be held. We later in the pause take the lock to get the current statistics, this change moves the update code to also happen while holding the lock. 
>> 
>> I've renamed `reset_statistics()` to `update_collection_stats()` to better match what it actually does and made it private. 
>> 
>> **Testing**
>> Mach5 tier1-5
>
> Stefan Johansson has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Additional blank line
>    
>    Co-authored-by: Stefan Karlsson <stefan.karlsson at oracle.com>
>  - Additional blank line
>    
>    Co-authored-by: Stefan Karlsson <stefan.karlsson at oracle.com>

lgtm. Just a comment about the assert message.

src/hotspot/share/gc/z/zPageAllocator.cpp line 1387:

> 1385:   }
> 1386: 
> 1387:   assert(total_used == _used, "Must be consistent at safepoint %zu == %zu", total_used, _used);

Preexisting, the assert message is misleading. Currently it is not the safepoint which guarantees consistency, but the page allocator lock. 

I wrote this assert at first under the assumption that we did not change _used concurrently with safepoints, but we did, so added the lock but forgot to update the assert text.


Maybe just removing the mention of safepoint is enough.
Suggestion:

  assert(total_used == _used, "Must be consistent %zu == %zu", total_used, _used);

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

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24719#pullrequestreview-2782868409
PR Review Comment: https://git.openjdk.org/jdk/pull/24719#discussion_r2053396904


More information about the hotspot-gc-dev mailing list