RFR: 8304412: Serial: Refactor old generation cards update after Full GC

[Thomas Schatzl]

On Fri, 17 Mar 2023 13:51:16 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Simple refactoring to use more informative names/comments.
> 
> Test: tier1-6

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/serial/cardTableRS.cpp line 134:

> 132: #endif
> 133: 
> 134: void CardTableRS::update_old_gen_cards(Generation* old_gen, bool is_young_gen_empty) {

Side note: It's unfortunate that we can't pass something more specific than `Generation*`.

src/hotspot/share/gc/serial/cardTableRS.cpp line 143:

> 141:     MemRegion prev_used_mr = old_gen->prev_used_region();
> 142:     if (used_mr.end() < prev_used_mr.end()) {
> 143:       // Shrinked; need to clear the previously-used but now-unused parts

Suggestion:

      // Shrunk; need to clear the previously-used but now-unused parts.

src/hotspot/share/gc/serial/genMarkSweep.cpp line 112:

> 110:   MarkSweep::_string_dedup_requests->flush();
> 111: 
> 112:   // Update old-gen cards to maintain old-to-young-pointer invariant

Suggestion:

  // Update old-gen cards to maintain old-to-young-pointer invariant.


This comment could be removed if the method were named better, see other comment.

src/hotspot/share/gc/serial/genMarkSweep.cpp line 114:

> 112:   // If compaction completely evacuated the young generation then we
> 113:   // can clear the card table.  Otherwise, we must invalidate
> 114:   // it (consider all cards dirty).  In the future, we might consider doing

I think this comment should be kept as description what `update_old_gen_cards` (which is a very generic name) does at some place. I.e. as documentation of that method.

Maybe something like `maintain_old_to_young_invariant` is better. It may not completely fit either but it is closer to what the method does.

E.g. "Updates old gen cards to maintain old-to-young-pointer invariant: Clears the old generation card table completely if the young generation had been completely evacuated, otherwise dirties the whole old generation to conservatively not loose any old-to-young pointer."

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

PR Review: https://git.openjdk.org/jdk/pull/13076#pullrequestreview-1354543721
PR Review Comment: https://git.openjdk.org/jdk/pull/13076#discussion_r1146161928
PR Review Comment: https://git.openjdk.org/jdk/pull/13076#discussion_r1146138729
PR Review Comment: https://git.openjdk.org/jdk/pull/13076#discussion_r1146178451
PR Review Comment: https://git.openjdk.org/jdk/pull/13076#discussion_r1146143315