On Thu, 26 Jun 2025 19:17:11 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:

>> This patch refines Parallel's sizing strategy to improve overall memory 
>> management and performance.
>> 
>> The young generation layout has been reconfigured from the previous 
>> `eden-from/to` arrangement to a new `from/to-eden` order. This new layout 
>> facilitates young generation resizing, since we perform resizing after a 
>> successful young GC when all live objects are located at the beginning of 
>> the young generation. Previously, resizing was often inhibited by live 
>> objects residing in the middle of the young generation (from-space). The new 
>> layout is illustrated in `parallelScavengeHeap.hpp`.
>> 
>> `NumberSeq` is now used to track various runtime metrics, such as 
>> minor/major GC pause durations, promoted/survived bytes after a young GC, 
>> highest old generation usage, etc. This tracking primarily lives in 
>> `AdaptiveSizePolicy` and its subclass `PSAdaptiveSizePolicy`.
>> 
>> GC overhead checking, which was previously entangled with adaptive resizing 
>> logic, has been extracted and is now largely encapsulated in 
>> `ParallelScavengeHeap::is_gc_overhead_limit_reached`.
>> 
>> ## Performance evaluation
>> 
>> - SPECjvm2008-Compress shows ~8% improvement on Linux/AArch64 and Linux/x64 
>> (restoring the regression reported in 
>> [JDK-8332485](https://bugs.openjdk.org/browse/JDK-8332485) and 
>> [JDK-8338689](https://bugs.openjdk.org/browse/JDK-8338689)).
>> - Fixes the surprising behavior when using a non-default (smaller) value of 
>> `GCTimeRatio` with Heapothesys/Hyperalloc, as discussed in [this 
>> thread](https://mail.openjdk.org/pipermail/hotspot-gc-dev/2024-November/050146.html).
>> - Performance is mostly neutral across other tested benchmarks: **DaCapo**, 
>> **SPECjbb2005**, **SPECjbb2015**, **SPECjvm2008**, and **CacheStress**. The 
>> number of young-gc sometimes goes up a bit and the total heap-size decreases 
>> a bit, because promotion-size-to-old-gen goes down with the more effective 
>> eden/survivor-space resizing.
>> 
>> PS: I have opportunistically set the obsolete/expired version to ~~25/26~~ 
>> 26/27 for now. I will update them accordingly before merging.
>> 
>> Test: tier1-8
>
> Albert Mingkun Yang has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains 25 commits:
> 
>  - Merge branch 'master' into pgc-size-policy
>  - review
>  - cast
>  - remove-young-resize-after-full-gc
>  - Merge branch 'master' into pgc-size-policy
>  - Merge branch 'master' into pgc-size-policy
>  - review
>  - Merge branch 'master' into pgc-size-policy
>  - merge
>  - version
>  - ... and 15 more: https://git.openjdk.org/jdk/compare/20e0055e...eeda1eb8

Changes requested by iwalulya (Reviewer).

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 354:

> 352: 
> 353: static bool check_gc_heap_free_limit(size_t free_bytes, size_t 
> capacity_bytes) {
> 354:   return free_bytes * 100 / capacity_bytes < GCHeapFreeLimit;

Suggestion:

  return (free_bytes * 100 / capacity_bytes) < GCHeapFreeLimit;

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 425:

> 423:   }
> 424: 
> 425:   if (check_gc_overhead_limit()) {

What is the effect of calling this method twice? Line 400 above, and then again 
here on line 425. Does that increment `_gc_overhead_counter` twice? More reason 
why i think the name is confusing.

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 756:

> 754: }
> 755: 
> 756: static size_t calculate_free_from_free_ratio_flag(size_t live, uintx 
> free_percent) {

Why refer to the `free_ratio_flag` instead of just 
`calculate_free_from_free_percent`?

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

PR Review: https://git.openjdk.org/jdk/pull/25000#pullrequestreview-2970469808
PR Review Comment: https://git.openjdk.org/jdk/pull/25000#discussion_r2174590762
PR Review Comment: https://git.openjdk.org/jdk/pull/25000#discussion_r2174637996
PR Review Comment: https://git.openjdk.org/jdk/pull/25000#discussion_r2174646534

Reply via email to