On Mon, 19 May 2025 06:00:09 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:

>> src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 363:
>> 
>>> 361: }
>>> 362: 
>>> 363: bool ParallelScavengeHeap::check_gc_overhead_limit() {
>> 
>> In main-line code, the method `check_gc_overhead_limit` is invoked by 
>> `PSScavenge::invoke` and `PSParallelCompact::invoke_no_policy` so that we 
>> can do the check after all the GCs. But now you only use 
>> `check_gc_overhead_limit` in 
>> `ParallelScavengeHeap::satisfy_failed_allocation`. I suspect whether it can 
>> check the gc overhead limit accurately.
>
>> so that we can do the check after all the GCs
> 
> Well, not really. In the old impl, 
> `GCOverheadChecker::check_gc_overhead_limit` calls 
> `set_gc_overhead_limit_exceeded` only for full-gc.
> 
>  > But now you only use check_gc_overhead_limit in 
> ParallelScavengeHeap::satisfy_failed_allocation. I suspect whether it can 
> check the gc overhead limit accurately.
> 
> I believe so.  In the old impl, we don't check gc-overhead for explicit gcs. 
> Only allocation-failure caused gcs are interesting, which all go through 
> `satisfy_failed_allocation`.
> 
> 
>   // Ignore explicit GC's.  Exiting here does not set the flag and
>   // does not reset the count.
>   if (GCCause::is_user_requested_gc(gc_cause) ||
>       GCCause::is_serviceability_requested_gc(gc_cause)) {
>     return;
>   }

`check_gc_overhead_limit` does more than `check`, can we find a more 
appropriate method name?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25000#discussion_r2174596366

Reply via email to