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

[William Kemper]

On Fri, 15 Nov 2024 14:11:10 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/shenandoahEvacTracker.cpp line 39:
> 
>> 37:   if (_use_age_table) {
>> 38:     _age_table = new AgeTable(false);
>> 39:   }
> 
> Sonar caught it: Initialize `_age_table` to `nullptr` for extra safety. Current uses seem to be gated by `_use_age_table`, though.

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

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 1856:
> 
>> 1854:     size_t consumed_old_collector = 0;
>> 1855:     size_t consumed_mutator = 0;
>> 1856:     size_t available_old = 0;
> 
> Sonar: `available_old` is not used.  `available_young` is not used.

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

> src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 380:
> 
>> 378: 
>> 379:   size_t old_evacuated = collection_set->get_old_bytes_reserved_for_evacuation();
>> 380:   size_t old_evacuated_committed = (size_t) (ShenandoahOldEvacWaste * old_evacuated);
> 
> Sonar: implicit conversion from 'size_t' (aka 'unsigned long') to 'double' may lose precision
> 
> Not sure if it breaks any math.

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

> src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.cpp line 656:
> 
>> 654:     // may not evacuate the entirety of unprocessed candidates in a single mixed evacuation.
>> 655:     const size_t max_evac_need = (size_t)
>> 656:             (old_generation()->unprocessed_collection_candidates_live_memory() * ShenandoahOldEvacWaste);
> 
> Sonar: implicit conversion from 'size_t' (aka 'unsigned long') to 'double' may lose precision
> 
> Since this is a heuristics calculation, should we be precise here?

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

> src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp line 89:
> 
>> 87: }
>> 88: 
>> 89: HeapWord* ShenandoahHeapRegion::allocate(size_t size, ShenandoahAllocRequest req) {
> 
> Sonar caught it: `ShenandoahAllocRequest` should be passed by reference here?

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

> src/hotspot/share/gc/shenandoah/shenandoahHeapRegionCounters.hpp line 99:
> 
>> 97:                       size_t region_size, size_t protocolVersion);
>> 98: 
>> 99:   uint _count = 0;
> 
> Sonar caught it: this `_count` field does not seem to be used.

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

> src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp line 157:
> 
>> 155:   size_t card_at_start = _rs->card_index_for_addr(address);
>> 156:   HeapWord* card_start_address = _rs->addr_for_card_index(card_at_start);
>> 157:   uint8_t offset_in_card = address - card_start_address;
> 
> Sonar: implicit conversion loses integer precision: 'long' to 'uint8_t' (aka 'unsigned char').
> 
> This looks risky. Should probably be `checked_cast<uint8_t>(pointer_delta(address, card_start_address, 1))`?

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

> src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp line 889:
> 
>> 887:   size_t previous_group_span = _group_entries[0] * _group_chunk_size[0];
>> 888:   for (size_t i = 1; i < _num_groups; i++) {
>> 889:     size_t previous_group_entries = (i == 1)? _group_entries[0]: (_group_entries[i-1] - _group_entries[i-2]);
> 
> Sonar: Value stored to `previous_group_entries` during its initialization is never read

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

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844218002
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844219136
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844219821
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844219644
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844218123
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844218269
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844219351
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1844219014