On Sat, 21 Jun 2025 20:56:37 GMT, Zhengyu Gu <z...@openjdk.org> wrote:
>> 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 19 commits: >> >> - review >> - Merge branch 'master' into pgc-size-policy >> - merge >> - version >> - Merge branch 'master' into pgc-size-policy >> - revert-aliases >> - Merge branch 'master' into pgc-size-policy >> - merge >> - merge-fix >> - merge >> - ... and 9 more: https://git.openjdk.org/jdk/compare/2b94b70e...a21e5363 > > src/hotspot/share/gc/parallel/parallelArguments.cpp line 71: > >> 69: // True in product build, since tests using debug build often stress GC >> 70: if (FLAG_IS_DEFAULT(UseGCOverheadLimit)) { >> 71: FLAG_SET_DEFAULT(UseGCOverheadLimit, trueInProduct); > > Given only Parallel honors `UseGCOverheadLimit`, you may want to set the > default to `trueInProduct` in `gc_globals.hpp` It's indeed that only Parallel implements this flag. However, if we decide to change the default value of this flag globally, I feel we should do that in its own PR for better visibility. Currently, this changes the default value only for Parallel, as one expects in a PR titled "Parallel: ...". > src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 381: > >> 379: HeapWord* result = young_gen()->expand_and_allocate(size); >> 380: >> 381: if (result == nullptr && !is_tlab && !should_alloc_in_eden(size)) { > > I feel that you changed intent of `should_alloc_in_eden()`. I believe the > original intent is to prevent allocating large objects in eden, and here, > seems to prevent allocating small objects in old gen. > > What benefits do you get? I don't see how I changed the semantics. On baseline, from `mem_allocate_work`: if (result != nullptr) { return result; } // If certain conditions hold, try allocating from the old gen. if (!is_tlab) { result = mem_allocate_old_gen(size); if (result != nullptr) { return result; } } and HeapWord* ParallelScavengeHeap::mem_allocate_old_gen(size_t size) { if (!should_alloc_in_eden(size)) { // Size is too big for eden. return allocate_old_gen_and_record(size); } return nullptr; } The original logic is essentially: if (result == nullptr && !is_tlab && !should_alloc_in_eden(size)) { // allocate in old-gen } which is the same as I put here. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25000#discussion_r2188594599 PR Review Comment: https://git.openjdk.org/jdk/pull/25000#discussion_r2188604946
