RFR(M): 8151085: Change G1 concurrent timer and tracer measuring time (original) (raw)

Bengt Rutisson bengt.rutisson at oracle.com
Mon Mar 14 12:17:51 UTC 2016

Hi Sangheon,

I like this cleanup. I think it makes things much more clear.

I'm fine with the behavioral changes regarding the timing. Thanks for listing them clearly in the table.

One minor nit:

g1CollectedHeap.cpp

2800 return G1HeapSummary(heap_summary, (Heap_lock->owned_by_self() ? used() : used_unlocked()), 2801 eden_used_bytes, eden_capacity_bytes, survivor_used_bytes, num_regions());

I would prefer to store (Heap_lock->owned_by_self() ? used() : used_unlocked()) in a local variable, just like eden_used_bytes, eden_capacity_bytes and survivor_used_bytes.

Other than that it looks good to me.

Thanks, Bengt

On 2016-03-11 20:48, sangheon wrote:

Hi all,

Could I have some reviews for this change? Concurrent gc timer and tracer are mainly used from concurrent mark thread except when abort happens. This abort results in a race condition between VM thread and concurrent mark thread. So I propose to do timer and tracer related works only from concurrent mark thread. Measurements are changed like below: Current Proposal Start timer and tracer When Young Initial Mark GC starts from VM thread When actually starts concurrent cycle from concurrent mark thread End timer and tracer 1. At the end of concurrent cycle 2. When abort happens At the end of concurrent cycle * I think this will be okay as even though we abort, we continue working for concurrent stuffs to finish concurrent work cycle. And we were just not measuring them. Heap summary (before-gc) At the beginning of Young Initial Mark GC. But concurrent mark thread is not working at that time. At the start of concurrent cycle. Heap summary (after-gc) 1. When abort happens before cleanup: sent when abort happens. 2. When abort happens after cleanup or normal case(w/o abort): sent at the end of cleanup. At the end of concurrent cycle which means it will include all minor clearing after 'Pause Cleanup'.

This proposal also includes related changes like below: 1. Removed the patch for JDK-8149834(race between VM thread and concurrent mark thread for concurrent gc timer): 8149834 was making related functions atomically while this proposal is changing not to happen that race. 2. Removed G1CollectorState::concurrentcyclestarted: no longer needed as related logics are running from concurrent mark thread. Currently 'total concurrent time' is not same as 'sum of concurrent phases' and the reason is that the concurrent timer is started when young initial mark gc happens. Secondly, concurrent mark thread is not counting the time for waiting VMThread to finish VM operation(SuspendibleThreadSet). I am planning to enhance this after '8151614: Improve logging in concurrent mark code' is pushed. CR: https://bugs.openjdk.java.net/browse/JDK-8151085 Webrev: http://cr.openjdk.java.net/~sangheki/8151085/webrev.00 Testing: JPRT, RBT(hotspotall, testlist: running now), local reproducer for JDK-8149834 Thanks, Sangheon

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

More information about the hotspot-gc-dev mailing list