On Tue, 8 Oct 2024 17:20:31 GMT, William Kemper <wkem...@openjdk.org> wrote:
>> This PR merges JEP 404, a generational mode for the Shenandoah garbage >> collector. The JEP can be viewed here: https://openjdk.org/jeps/404. We >> would like to target JDK24 with this PR. > > William Kemper has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 478 commits: > > - Fix merge error > - Merge remote-tracking branch 'jdk/master' into great-genshen-pr-redux > - Merge remote-tracking branch 'jdk/master' into great-genshen-pr-redux > - Merge branch 'shenandoah/master' into great-genshen-pr-redux > - Merge > - 8341099: GenShen: assert(HAS_FWD == _heap->has_forwarded_objects()) > failed: Forwarded object status is sane > > Reviewed-by: kdnilsen > - 8341485: GenShen: Make evac tracker a non-product feature and confine it > to generational mode > > Reviewed-by: kdnilsen, ysr > - Merge > - 8341042: GenShen: Reset mark bitmaps for unaffiliated regions when > preparing for a cycle > > Reviewed-by: kdnilsen > - 8339616: GenShen: Introduce new state to distinguish promote-in-place > phase as distinct from concurrent evacuation > > Reviewed-by: kdnilsen, shade, ysr > - ... and 468 more: https://git.openjdk.org/jdk/compare/b9db74a6...4db1e0e1 I looked at the remaining changes, it's mostly tests. Only some minor comments/requests on those. test/hotspot/jtreg/gc/shenandoah/TestAllocIntArrays.java line 100: > 98: > 99: /* > 100: * @test id=generational You are making a test configuration and call it 'generational' but then one of the two run configurations doesn't actually run with generational mode. You probably want to separate the two? test/hotspot/jtreg/gc/shenandoah/TestAllocIntArrays.java line 163: > 161: final int min = 0; > 162: final int max = 384 * 1024; > 163: // Each allocated int array is assumed to consume 16 bytes for > alignment and header, plus With the upcoming 'compact headers' it's going to be only 12 bytes. If that matters, then the actual number should perhaps be a constant? Preexisting and not relevant for this PR, though. test/hotspot/jtreg/gc/shenandoah/oom/TestAllocLargeObj.java line 1: > 1: /* Why are you removing the test? test/hotspot/jtreg/gc/shenandoah/oom/TestAllocLargerThanHeap.java line 1: > 1: /* And this test? test/hotspot/jtreg/gc/shenandoah/oom/TestAllocOutOfMemory.java line 1: > 1: /* Or is this new test subsuming several old tests? ------------- Changes requested by rkennke (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21273#pullrequestreview-2362848068 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1796980759 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1796983485 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1797098313 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1797098581 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1797099580