RFR (S): JDK-8076241: Remove unused methods mod_card_iterate() and non_clean_card_iterate_serial()

[Bengt Rutisson]

Hi Kim,

On 2015-04-01 01:59, Kim Barrett wrote:
> On Mar 31, 2015, at 2:59 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>> On 2015-03-31 00:16, Kim Barrett wrote:
>>> ------------------------------------------------------------------------------
>>> src/share/vm/memory/cardTableModRefBS.hpp
>>>   181   // XXX ??? MemRegionClosure above vs OopsInGenClosure below XXX
>>>   182   // XXX some new_dcto_cl's take OopClosure's, plus as above there are
>>>   183   // some MemRegionClosures. Clean this up everywhere. XXX
>>>
>>> This untouched comment is referring to the removed
>>> non_clean_card_iterate_serial function ("above"), so needs to be
>>> updated. It may be that the comment can be removed entirely now,
>>> possibly being moved to the technical debt wiki.
>> Ah. Good point. I removed the comment and added this to the technical debt wiki:
>>
>> "CardTableModRefBS::dirty_card_iterate() works with a MemRegion and a MemRegionClosure. Other similar methods, such as CardTableModRefBS::non_clean_card_iterate_possibly_parallel() and CardTableModRefBS::process_stride() use OopsInGenClosure instead. Can we use a consistent API?”
> That works for me.
>
>>> ------------------------------------------------------------------------------
>>> src/share/vm/memory/cardTableModRefBS.hpp
>>>   382   // Invoke "cl.do_MemRegion" on a set of MemRegions that collectively
>>>   383   // includes all the modified cards (expressing each card as a
>>>   384   // MemRegion).  Thus, several modified cards may be lumped into one
>>>   385   // region.  The regions are non-overlapping, and are visited in
>>>   386   // *decreasing* address order.  (This order aids with imprecise card
>>>   387   // marking, where a dirty card may cause scanning, and summarization
>>>   388   // marking, of objects that extend onto subsequent cards.)
>>>
>>> This comment was the reason I'd deferred removing mod_card_iterate. So
>>> far as I can tell, this comment is the *only* place where there is any
>>> discussion whatsoever of why some card iterators scan backward. I
>>> wanted to find a new and better home for that information before
>>> deleting the comment.
>> I see. I couldn't find a card iterator that scans backwards now. Do you have a pointer to one that still scans backwards?
>>
>> I'll push the change with the comment removed. I'm happy to find a new place for the comment if we have an iterators that scan backwards.
> I think the only one left is ClearNoncleanCardWrapper::do_MemRegion().  But I don’t guarantee there aren’t more.

Thanks for pointing this out.

As I mentioned before I'm happy to push a fix to add the comment back. 
Do you think this would be appropriate?

diff --git a/src/share/vm/memory/cardTableRS.cpp 
b/src/share/vm/memory/cardTableRS.cpp
--- a/src/share/vm/memory/cardTableRS.cpp
+++ b/src/share/vm/memory/cardTableRS.cpp
@@ -176,7 +176,10 @@
  bool ClearNoncleanCardWrapper::is_word_aligned(jbyte* entry) {
    return (((intptr_t)entry) & (BytesPerWord-1)) == 0;
  }
-
+// The regions are visited in *decreasing* address order.
+// This order aids with imprecise card marking, where a dirty
+// card may cause scanning, and summarization marking, of objects
+// that extend onto subsequent cards.
  void ClearNoncleanCardWrapper::do_MemRegion(MemRegion mr) {
    assert(mr.word_size() > 0, "Error");
    assert(_ct->is_aligned(mr.start()), "mr.start() should be card 
aligned");

>
> I was trying to build a list of all the card table iterators as part of working on
> https://bugs.openjdk.java.net/browse/JDK-8060244
> There were a surprising number, all different; and I’m not sure I found them all either.

Nice summary in the bug report! I like the idea of making the card 
iteration more efficient and trying to merge the implementations. Do you 
have plans to work on this?

Thanks,
Bengt

>