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

[John Cuthbertson]

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/20120625/b492d257/attachment.htm>