RFR: 8339870: Remove yet more stale TODOs

[Y. Srinivas Ramakrishna]

On Tue, 10 Sep 2024 17:12:13 GMT, William Kemper <wkemper at openjdk.org> wrote:

> Final round of TODO removal.
> * Move asterisk for pointer next to type, not variable
> * Remembered set verifier uses a small 'scanner' abstraction to simplify reading of read or write card table
> 
> ## Testing
> 
> GHA, Internal pipelines

Looks good. Thanks for the cleanups!

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp line 687:

> 685:     _generation->prepare_regions_and_collection_set(true /*concurrent*/);
> 686: 
> 687:     // Upon return from prepare_regions_and_collection_set(), certain parameters have been established to govern the

For the block comment at lines 687-702 (right pane), would it make sense to move it to the spec of `prepare_...` and remove this block comment from here?

src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 736:

> 734: //  of memory.  As currently implemented, all regions are compacted toward the low-end of memory.  This creates more
> 735: //  fragmentation of the heap, because old-gen regions get scattered among low-address regions such that it becomes
> 736: //  more difficult to find contiguous regions for humongous objects.

Is this covered in a ticket?

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp line 728:

> 726:         assert(_old_heuristics->coalesce_and_fill_candidates_count() > 0, "Expected coalesce and fill candidates");
> 727:         // When the heuristic put the old generation in this state, it didn't know
> 728:         // that we would unload classes and make everything parseable. But, we know

parsable

src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp line 91:

> 89: }
> 90: 
> 91: bool ShenandoahDirectCardMarkRememberedSet::is_write_card_dirty(HeapWord* p) const {

Interesting. Is this new? Is it used somewhere or introduced for the purpose of a more consistent/uniform API?

(Not requiring any change; just asking. I am all for uniform/consistent APIs.)

src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp line 324:

> 322: }
> 323: 
> 324: bool ShenandoahScanRemembered::is_write_card_dirty(HeapWord* p) {

Same question/remark as I asked/made for the ShenDirectCardMarkRS api above at line 91.

src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp line 872:

> 870:   size_t _chunk_offset;          // HeapWordSize offset
> 871:   size_t _chunk_size;            // HeapWordSize qty
> 872: };

At line 880 below:

"other worker will threads repeatedly ..." should be "other worker threads will repeatedly".

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

Marked as reviewed by ysr (Committer).

PR Review: https://git.openjdk.org/shenandoah/pull/496#pullrequestreview-2293570817
PR Review Comment: https://git.openjdk.org/shenandoah/pull/496#discussion_r1752598856
PR Review Comment: https://git.openjdk.org/shenandoah/pull/496#discussion_r1752628748
PR Review Comment: https://git.openjdk.org/shenandoah/pull/496#discussion_r1752631946
PR Review Comment: https://git.openjdk.org/shenandoah/pull/496#discussion_r1752652646
PR Review Comment: https://git.openjdk.org/shenandoah/pull/496#discussion_r1752657839
PR Review Comment: https://git.openjdk.org/shenandoah/pull/496#discussion_r1752667945