RFR: 8371465: Parallel: Revise asserts around heap expansion

Axel Boldt-Christmas aboldtch at openjdk.org
Thu Nov 13 08:21:14 UTC 2025


On Wed, 12 Nov 2025 13:14:34 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Revise heap/old-gen expansion asserts/comments to cover two paths:
> 
> 1. startup: no gc can occur
> 2. after-startup: during safepoint, by vm-thread.
> 
> Test: tier1-4

Alright so if I try to understand the invariants correct here surrounding the `_end`.

For using unordered reads and modifying the value:
  * Before GC can run
    * Mapping AOT archive has no concurrency
      * Will take `PSOldGenExpand_lock` unnecessarily, but not the `Heap_lock`
      * assert_locked_or_safepoint(Heap_lock) is skipped because `!Universe::is_fully_initialized()`
    * Normal failed allocations (may be while loading Streamed AOT archive)
      * Holds the `Heap_lock`
  * After the GC can run
    * In the VM thread of a GC safepoint (no parallel GC threads are running)
    * By a GC thread during a GC
      * Uses `PSOldGenExpand_lock`

Looking at the code and writing this out I can see that only half of my concerns raised in #28162 / [JDK-8371369](https://bugs.openjdk.org/browse/JDK-8371369) were addressed. 

The asserts in `MutableSpace::needs_expand` were addressed, but not the once in `PSOldGen::expand`. Right now if we are in no GC phase (`!is_init_completed()`) and we would fail to expand the young gen, and attempt to expand the old gen we would hit the assert inside `PSOldGen::expand` which says that any thread other than the VM thread must also hold `PSOldGenExpand_lock`. Which we do not in the allocation path. 

My guess is that this scenario is extremely unlikely to happen. However I think the assert inside `PSOldGen::expand` should be conditioned on `if (!Thread::current()->is_VM_thread() && !is_init_completed())`.

Other than that I think these new invariants assert looks good. But some of the interactions between `PSOldGenExpand_lock`, `Heap_lock` are not obvious at a glance.

May we should have some comment (or maybe there already is) describing how this works.

Something along the lines of:

Before `is_init_completed()` all expansion is performed while holding the `Heap_lock`. 
With the exception of when the AOT heap archive is mapped into the heap. 
After `is_init_completed()` all expansions are either preformed by active GC threads holding the `PSOldGenExpand_lock` or the VM thread in a GC safepoint. The GC threads' and the VM thread's expansion will never occur concurrently.

_Note here we might just want to take the `Heap_lock` in `ParallelScavengeHeap::allocate_loaded_archive_space` and keep this invariant, just because it simplifies the invariant and we can remove the mapped AOT archive exception line._

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

PR Review: https://git.openjdk.org/jdk/pull/28266#pullrequestreview-3458284737


More information about the hotspot-gc-dev mailing list