On Wed, 13 Nov 2024 19:32:53 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 510 commits: > > - Merge branch 'merge-latest' into great-genshen-pr-redux > - Use new CompactHeader forwarding APIs in generational mode > - Merge remote-tracking branch 'jdk/master' into merge-latest > - Merge > - 8343649: Shenandoah: ShenandoahEvacInfo event does not follow JFR > guidelines > > Reviewed-by: wkemper > - Merge > - 8343227: GenShen: Fold resource mark into management of preselected regions > > Reviewed-by: kdnilsen > - Merge openjdk/jdk tip into great-genshen-pr-redux > - Merge remote-tracking branch 'jdk/master' into merge-latest > - Merge remote-tracking branch 'jdk/master' into merge-latest > - ... and 500 more: https://git.openjdk.org/jdk/compare/889f9062...5e02b5d8 More reviews... src/hotspot/share/gc/shenandoah/shenandoahGenerationSizer.cpp line 141: > 139: dst->increase_capacity(bytes_to_transfer); > 140: const size_t new_size = dst->max_capacity(); > 141: log_info(gc)("Transfer " SIZE_FORMAT " region(s) from %s to %s, > yielding increased size: " PROPERFMT, This should not be `log_info(gc)`. In fact, please look at other places where you print logging. Generally, `log_info(gc)` should provide _only_ the high-level GC info: which phases were running. Everything else should go under `log_debug(gc)`. src/hotspot/share/gc/shenandoah/shenandoahSTWMark.cpp line 140: > 138: break; > 139: } > 140: case OLD: Are we not doing OLD STW mark? Deserves a comment. src/hotspot/share/gc/shenandoah/shenandoahYoungGeneration.hpp line 54: > 52: void parallel_heap_region_iterate(ShenandoahHeapRegionClosure* cl) > override; > 53: > 54: void parallel_region_iterate_free(ShenandoahHeapRegionClosure* cl) > override; Is this a sibling of `parallel_heap_region_iterate`? Shouldn't these be called `parallel_heap_region_iterate_free`? ------------- PR Review: https://git.openjdk.org/jdk/pull/21273#pullrequestreview-2436882261 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1842779639 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1842701030 PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1842704295
