RFR(S): 8173335: Improve logging for j.l.ref.reference processing

sangheon sangheon.kim at oracle.com
Tue Jun 13 00:13:21 UTC 2017


Hi Aleksey,

Thanks for the review.

On 06/12/2017 09:06 AM, Aleksey Shipilev wrote:
> On 06/10/2017 01:57 AM, sangheon wrote:
>> CR: https://bugs.openjdk.java.net/browse/JDK-8173335
>> webrev: http://cr.openjdk.java.net/~sangheki/8173335/webrev.0
> Oh, good! I had to instrument these by hand when optimizing RP paths.
>
> Comments after brief look:
>
>   *) So, the path with NULL executor are also not handling the timer? E.g. CMS:
>
>   5262   if (rp->processing_is_mt()) {
>   5263     rp->balance_all_queues();
>   5264     CMSRefProcTaskExecutor task_executor(*this);
>   5265     rp->enqueue_discovered_references(&task_executor, _gc_timer_cm);
>   5266   } else {
>   5267     rp->enqueue_discovered_references(NULL);
>   5268   }
Fixed to use timers for similar cases that you pointed. Thanks for 
catching up this!
I started this CR as a part of MT ref. processing(JDK-8043575), so I 
only added to that path. But this should be fixed.

>
>   *) I would leave "Ref Counts" line as usual for compatibility reasons. Changing
> it to "Counts" would force GC log parsers to handle that corner case too.
Changed, 'Counts -> Ref Counts'.

>
>   *) This may reuse Indents?
>
>     95       out->print("%s", "    ");
Fixed to use Indents[2].

>
>   *) Probably makes sense to "hg mv -A" the workerDataArray files to preserve the
> Mercurial history -- webrev should say something like "copied from ...", IIRC.
Fixed.

webrev:
http://cr.openjdk.java.net/~sangheki/8173335/webrev.1/
http://cr.openjdk.java.net/~sangheki/8173335/webrev.1_to_0

Thanks,
Sangheon


>
> Thanks,
> -Aleksey
>




More information about the hotspot-gc-dev mailing list