RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics [v8]

[Thomas Schatzl]

On Thu, 3 Apr 2025 07:28:13 GMT, Man Cao <manc at openjdk.org> wrote:

> Re: concurrency issue with reading `SoftMaxHeapSize`
> 
> I updated to `Atomic::load()`, but not sure if I understand the concern correctly.
> 
> > So e.g. the assignment of `G1IHOPControl::get_conc_mark_start_threshold` to `marking_initiating_used_threshold` in that call can be inlined in `G1Policy::need_to_start_conc_mark` (called by the mutator in `G1CollectedHeap::attempt_allocation_humongous`) in multiple places, and so `SoftMaxHeapSize` re-read with multiple different values in that method.
> 
> I don't see where the re-read is. I think in any code path from `G1IHOPControl::get_conc_mark_start_threshold`, `G1CollectedHeap::heap()->soft_max_capacity()` is called only once. `G1CollectedHeap::attempt_allocation_humongous` also appears to call `G1Policy::need_to_start_conc_mark` only once, which calls `G1IHOPControl::get_conc_mark_start_threshold` only once.
> 
> I agree it is a data race if `soft_max_capacity()` runs outside of a safepoint, so `Atomic::load()` makes sense regardless.

The compiler could be(*) free to call `get_conc_mark_start_threshold()` again in any of the uses of the local variable without telling it that one of its components may change between re-reads.

(*) Probably not after looking again, given that it's not marked as `const` (not sure why), and a virtual method, and fairly large.

The situation would be much worse if somehow `SoftMaxHeapsize` could be changed within a safepoint.

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

PR Comment: https://git.openjdk.org/jdk/pull/24211#issuecomment-2774885501