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

[Bengt Rutisson]

Hi Kim,

Thanks, for looking at this!

On 2015-03-31 00:16, Kim Barrett wrote:
> On Mar 30, 2015, at 10:18 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>>
>> Hi all,
>>
>> Could I have a couple of reviews for this small change?
>>
>> http://cr.openjdk.java.net/~brutisso/8076241/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8076241
>>
>> The methods mod_card_iterate() and non_clean_card_iterate_serial() in CardTableModRefBS are not being used.
>>
>> Thanks,
>> Bengt
> Aww. I was going to do that. Actually, I was supposed to file a RFE
> for this as a followup to 8062206 (Remove unusable
> G1RSLogCheckCardTable command line argument), but that seems to have
> not gotten out of my notes file.

I see. No problem. I just ran in to this since 
non_clean_card_iterate_serial() makes use of SharedHeap. Rather than 
trying to updated it when I remove SharedHeap I'd like to remove the 
dead code.

>
> ------------------------------------------------------------------------------
> 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?"

>
> ------------------------------------------------------------------------------
> 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.

>
> ------------------------------------------------------------------------------
>
> The code changes look good.

Good. Thanks!

Bengt

>