Request for review (XS): 7036482: clear argument is redundant and unused in cardtable methods

Y. Srinivas Ramakrishna y.s.ramakrishna at oracle.com
Thu Apr 14 08:39:48 UTC 2011


Thanks for the review John!

On 4/14/2011 1:10 AM, John Coomes wrote:
> Y. S. Ramakrishna (y.s.ramakrishna at oracle.com) wrote:
>>
>> 7036482: clear argument is redundant and unused in cardtable methods
>>
>> http://cr.openjdk.java.net/~ysr/7036482/webrev.00/
>
> The change in allocationStats.hpp is unrelated, is it intended for
> this fix?  Aside from that, looks good to me.

I'll let you and Bengt vote on it. Left to myself I'd sneak it
in, although I agree that it's unrelated to the synopsis here.
(May be i could file a separate bug and include this change also,
or I could just mention it in the summary, as an aside.)
Let me know which you prefer, or if you'd prefer i just dropped
that change.

>
>> As in synopsis; further motivational detail may be found
>> in the CR. Some further cleanups/deletions will follow in
>> subsequent CR's to be filed.
>
> mod_oop_in_space_iterate(), the only method that calls
> non_clean_card_iterate() with clear != false, is unused.  Is deleting
> it one of the further cleanups?  Removing it makes it easier to see
> that 'clear' is not needed, so you could include it with this change.

I initially considered deleting mod_oop_in_space_iterate(), but then
thought it was useful as an existing interface, rather along the lines of
mod_card_iterate(). I did remove the clear arg to
it, so that a caller would have to use a "clearing wrapper" around the
closure if they wanted the cards cleared. (This would hold for
other interfaces which were modified in this manner too, such
as mod_card_iterate()).

I have a few more cleanups coming, so I will see if the future cleanups
tell me that mod_oop_in_space_iterate() should go away. I am not
quite sure at the moment if it should, although given that it
has been around for so long and is unused should mean that it's
useless: may be that should be reason enough to delete it.
OK, I'll delete it too :->

Thanks for your review!
-- ramki


>
> -John




More information about the hotspot-gc-dev mailing list