RFR(S): JDK-8040002: Clean up code and code duplication in re-diryting cards for verification

[Jon Masamitsu]

On 4/16/2014 1:59 AM, Thomas Schatzl wrote:
> Hi Jon,
>
> On Tue, 2014-04-15 at 07:24 -0700, Jon Masamitsu wrote:
>> Thomas,
>>
>> http://cr.openjdk.java.net/~tschatzl/8040002/webrev/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.frames.html
>>
>> Changes look good.
>>
>> Just a clarification.
>>
>> So the is_in_reserved() test was an attempt to
>> make do_card_ptr() faster?
>>
>>    163   bool do_card_ptr(jbyte* card_ptr, uint worker_i) {
>>    164     if (_g1h->is_in_reserved(_ctbs->addr_for(card_ptr))) {
>>    165       _calls++;
>>    166       *card_ptr = 0;
>>    167     }
>>    168     return true;
>>    169   }
>>
>>
>> And the assertion checking that addr_for() does on the
>> address card_ptr is not needed?
> Actually I think the whole check is superfluous, also in jdk7.
> Is_in_reserved() checks whether the card is in the heap (including perm
> gen), and addr_for() also checks the same before and after converting
> the card address to a heap address.
>
> However cards can only ever be in the heap anyway - if they are not,
> this is an error.
>
> This check is almost like an instance of of "if (something_true) {...}
> else { guarantee(!false, "paranoia"); }" seen in other places in G1
> code.

Yes, it has the feel of paranoia.

>
> I can add an assert here if you think it is useful.

No need for assertion checking.  If something is wrong, it should break
in an obvious way and we're getting too slow in fastdebug builds as
it is.

Jon

>
> Thomas
>
>