RFR: 8304144: G1: Remove unnecessary is_survivor check in G1ClearCardTableTask

Y. Srinivas Ramakrishna ysr at openjdk.org
Mon Mar 20 23:58:40 UTC 2023


On Mon, 20 Mar 2023 11:14:18 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> "dirty" in "dirty_young_block" can be misleading. It's like "annotation" as it essentially calls `g1_mark_as_young`, which ensures writes to young-gen doesn't trigger the barrier slow path (e.g. `G1BarrierSet::write_ref_field_post`).
> 
> In other places, "dirty" has a reserved meaning that the heap mem range for a dirty card potentially contains an old-to-young pointer that must be scanned.
> 
> It's probably better to rename this method to sth less misleading in another PR.

I don't know about renaming, but `clean` / `clear` seems to be unequivocal in its intentions and actions:


void HeapRegion::clear_cardtable() {
  G1CardTable* ct = G1CollectedHeap::heap()->card_table();
  ct->clear(MemRegion(bottom(), end()));
}

and which seems to eventually bottom out in:

void CardTable::clear_MemRegion(MemRegion mr) {
  // Be conservative: only clean cards entirely contained within the
  // region.
  CardValue* cur;
  if (mr.start() == _whole_heap.start()) {
    cur = byte_for(mr.start());
  } else {
    assert(mr.start() > _whole_heap.start(), "mr is not covered.");
    cur = byte_after(mr.start() - 1);
  }
  CardValue* last = byte_after(mr.last());
  memset(cur, clean_card, pointer_delta(last, cur, sizeof(CardValue)));
}


But clearing (i.e. marking card clean) today, from my cursory reading of the code, over-writes the `g1_young_gen` value, because:


    g1_young_gen = CT_MR_BS_last_reserved << 1,

(etc.)

(i.e. the value `0x10` see below)
while:


  enum CardValues {
    clean_card                  = (CardValue)-1,

    dirty_card                  =  0,
    CT_MR_BS_last_reserved      =  1
  };


I didn't check the pointer update enqueueing code to see if what it was doing though. And don't know if this materially affects performance, but may be @tschatzl or someone more familiar with G1's recent evolution might know.

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

PR Comment: https://git.openjdk.org/jdk/pull/13023#issuecomment-1477099469


More information about the hotspot-gc-dev mailing list