Response from ###@###.###: ------------------------------- ... Thanks. Your fix makes more sense than mine. Response from ###@###.###: -------------------------------------- Hi Neo, Thanks for the heads-up. This seems worth fixing, at least for hygiene, How about the following instead, where we avoid the addr_for() translation until we are sure it will be safe, provided that the mr argument to the method is backed by card table entries. This avoids the need for the extra compare-and-branch inside the loop in your patch. void do_MemRegion(MemRegion mr) { HeapWord* end_of_non_clean = mr.end(); HeapWord* start_of_non_clean = end_of_non_clean; jbyte* entry = _ct->byte_for(mr.last()); jbyte* first_entry = _ct->byte_for(mr.start()); while (entry >= first_entry) { jbyte entry_val = *entry; HeapWord* cur = _ct->addr_for(entry); if (!clear_card(entry)) { if (start_of_non_clean < end_of_non_clean) { MemRegion mr2(start_of_non_clean, end_of_non_clean); _dirty_card_closure->do_MemRegion(mr2); } end_of_non_clean = cur; start_of_non_clean = end_of_non_clean; } start_of_non_clean = cur; entry--; } if (start_of_non_clean < end_of_non_clean) { MemRegion mr2(start_of_non_clean, end_of_non_clean); _dirty_card_closure->do_MemRegion(mr2); } } Email from ###@###.###: [OpenJDK] ---------------------------- hi, Acturally, the existing code works well although it misses the limitation checking, because the current heap layout will make sure the memory region checking failed before it goes beyond the limitation of byte map. The reason is that the nursery space is on the lower address, which will make its committed card table also on the lower address of inside the byte maps. While walking through the card table of the mature space and keeping reduce the entry, we will finally hit the boundary of its byte map before doing the check of memory region, which will fire the assert of the addr_for, if it is on the lower address. So, just adding more safety for this function. Thanks, Neo Index: memory/cardTableRS.cpp =================================================================== --- memory/cardTableRS.cpp (revision 86) +++ memory/cardTableRS.cpp (working copy) @@ -163,6 +163,9 @@ start_of_non_clean = end_of_non_clean; } entry--; + // Adding a limit checking for safety. + if (entry < _ct->base_map()) + break; start_of_non_clean = cur; cur = _ct->addr_for(entry); } Index: memory/cardTableRS.hpp =================================================================== --- memory/cardTableRS.hpp (revision 86) +++ memory/cardTableRS.hpp (working copy) @@ -101,6 +101,8 @@ CardTableModRefBS* ct_bs() { return &_ct_bs; } + jbyte * base_map() { return &(_ct_bs._byte_map[0]); } + // Override. void prepare_for_younger_refs_iterate(bool parallel);
|