RFR(M): 8151085: Change G1 concurrent timer and tracer measuring time
stefan.johansson at oracle.com
Tue Mar 15 13:52:41 UTC 2016
On 2016-03-14 20:12, sangheon wrote:
> Hi Bengt,
> Thank you for reviewing this.
> On 03/14/2016 05:17 AM, Bengt Rutisson wrote:
>> 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.
I also like this, it's a big improvement.
>> One minor nit:
>> 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.
> Here's updated webrev.
> http://cr.openjdk.java.net/~sangheki/8151085/webrev.01_to_00 (inc)
> FYI: test result of RBT for hotspot_all and testlist is OK.
>> 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:
>>> 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::_concurrent_cycle_started: 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
>>> 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(hotspot_all, testlist: running now), local
>>> reproducer for JDK-8149834
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev