RFR(M): 6921087: G1: remove per-GC-thread expansion tables from the fine-grain remembered sets

[Bengt Rutisson]

Hi Thomas and John,

The latest patch looks good to me too. Thomas, thanks a lot for cleaning 
this up!

Bengt

On 2012-06-25 21:10, John Cuthbertson wrote:
> Hi Thomas,
>
> Thanks. It looks good to me. I'll be pushing it later today.
>
> JohnC
>
> On 06/25/12 08:43, Thomas Schatzl wrote:
>> Hi John,
>>
>> On Fri, 2012-06-22 at 11:38 -0700, John Cuthbertson wrote:
>>    
>>> Hi Thomas,
>>>
>>> I would rather not remove the HRRS_VERBOSE code. It has proven to be
>>> invaluable in the past when debugging the dreaded "missing entry in
>>> remembered set" failure. So as potential candidate for investigating
>>> an error like this in the future - I would like to keep the code in
>>> place rather than recreate it. I'm OK with not using conditional
>>> compilation and using a 'develop' flag as suggested by Bengt.
>>>
>>> Anyway a new webrev based upon your latest patch can be found at:
>>> http://cr.openjdk.java.net/~johnc/6921087/webrev.1/
>>>      
>>
>>    either way is fine with me, although to me it seems to classical debug
>> code that has been used once ages ago for tracking down a particular bug
>> and kept in "just in case".
>> Imo, while it is a small time saver when/if such a bug occurs again, for
>> me the time spent adding debug log output has never been an issue when
>> tracking down such things.
>>
>> If you intend to keep it, please consider fixing it though, so that the
>> code compiles when HRRS_VERBOSE is enabled: in
>> PerRegionTable::add_reference_work there is a dereference of the "from"
>> OopOrNarrowOopStar, i.e. I believe
>>
>>    void add_reference_work(OopOrNarrowOopStar from, bool par) {
>>      // Must make this robust in case "from" is not in "_hr", because of
>>      // concurrency.
>>
>> #if HRRS_VERBOSE
>>      gclog_or_tty->print_cr("    PRT::Add_reference_work(" PTR_FORMAT "->" PTR_FORMAT").",
>>                             from, *from);
>> #endif
>>
>> should read something like:
>>
>> #if HRRS_VERBOSE
>>      gclog_or_tty->print_cr("    PRT::Add_reference_work(" PTR_FORMAT "->" PTR_FORMAT").",
>>                             from,
>>                             UseCompressedOops
>>                             ? oopDesc::load_decode_heap_oop((narrowOop*)from)
>>                             : oopDesc::load_decode_heap_oop((oop*)from));
>> #endif
>>
>> (I think)
>>
>> I attached a patch that changes the HRRS_VERBOSE define to a G1TraceHeapRegionRememberedSet develop variable, to see if a debug variable is the better solution. At least this avoids compilation errors like above creeping in.
>>
>> Hth,
>>    Thomas
>>
>>    
>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120626/8c2ef8ad/attachment.htm>