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