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

[sangheon]

Hi Stefan,

Thank you for reviewing this!

Sangheon


On 03/15/2016 06:52 AM, Stefan Johansson wrote:
> Hi Sangheon,
>
> 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.
>
>> Thanks!
>>
>>>
>>> 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.
>> Here's updated webrev.
>>
>> http://cr.openjdk.java.net/~sangheki/8151085/webrev.01
>> http://cr.openjdk.java.net/~sangheki/8151085/webrev.01_to_00 (inc)
>>
> Looks good.
>
> Thanks,
> Stefan
>> FYI: test result of RBT for hotspot_all and testlist is OK.
>>
>> Thanks,
>> Sangheon
>>
>>
>>>
>>> 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::_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 
>>>> 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(hotspot_all, 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/20160315/f834d4bf/attachment.htm>