RFR: 8337511: Implement JEP 404: Generational Shenandoah (Experimental) [v7]

William Kemper wkemper at openjdk.org
Fri Nov 15 01:27:59 UTC 2024


On Thu, 14 Nov 2024 19:26:12 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> 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
>
> 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)`.

https://bugs.openjdk.org/browse/JDK-8344263

> 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.

https://bugs.openjdk.org/browse/JDK-8344264

> 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`?

https://bugs.openjdk.org/browse/JDK-8344264

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1843073877
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1843075157
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1843075212


More information about the shenandoah-dev mailing list