[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: PING: RFR: JDK-8072913: [REDO] GCCause should distinguish jcmd GC.run from System.gc()
From:       Yasumasa Suenaga <yasuenag () gmail ! com>
Date:       2015-05-28 14:03:05
Message-ID: 55672019.7070406 () gmail ! com
[Download RAW message or body]

> I'll sponsor it.

Thank you Jesper!
I will send a chengeset after reviewing.


Yasumasa


On 2015/05/28 22:47, Jesper Wilhelmsson wrote:
> Looks good!
> I'll sponsor it.
> /Jesper
>
> Yasumasa Suenaga skrev den 28/5/15 05:31:
>> I've uploaded new webrev:
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.02/
>>
>> I need a Sponsor and more reviewer.
>> Please review it.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2015/05/28 8:52, Yasumasa Suenaga wrote:
>>> Hi Jesper,
>>>
>>> Thank you for your comment.
>>> I will fix it.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2015/05/28 5:14, Jesper Wilhelmsson wrote:
>>>> Hi,
>>>>
>>>> I like that you removed _jvmti_force_gc from is_user_requested_gc() and used
>>>> this method throughout. It is cleaner and is_user_requested_gc() makes more
>>>> sense now.
>>>>
>>>> In vmCMSOperations.cpp I think the comment should say GCCause::_dcmd_gc_run.
>>>>
>>>> Besides that minor comment, looks good!
>>>>
>>>> Thanks,
>>>> /Jesper
>>>>
>>>>
>>>> Yasumasa Suenaga skrev den 20/4/15 15:53:
>>>>> Hi all,
>>>>>
>>>>> I've uploaded webrev for this enhancement.
>>>>> Could you review it?
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2015/03/11 22:13, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>>> So I think we can remove _jvmti_force_gc from is_user_requested_gc() and add
>>>>>>> _dcmd_gc_run
>>>>>>> to it.
>>>>>>
>>>>>> I've uploaded new webrev, and I've applied it to new patch.
>>>>>> Could you review it?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/
>>>>>>
>>>>>> I also updated jtreg testcase.
>>>>>> It works fine in my environment.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2015/02/14 22:10, Yasumasa Suenaga wrote:
>>>>>>> Hi Mikael,
>>>>>>>
>>>>>>>> I'd prefer if you could add a GCCause::is_system_gc_equivalent() which
>>>>>>>> returns true for some set of GCCause enum values, such as
>>>>>>>> _java_lang_system_gc and _dcmd_gc_run
>>>>>>>
>>>>>>> Can I add _dcmd_gc_run to GCCause::is_user_requested_gc() ?
>>>>>>> This function is used with GCCause::is_serviceability_requested_gc() .
>>>>>>> CMSCollector::is_external_interruption() and
>>>>>>> AdaptiveSizePolicy::check_gc_overhead_limit()
>>>>>>>
>>>>>>> is_user_requested_gc() and is_serviceability_requested_gc() checkes
>>>>>>> _jvmti_force_gc
>>>>>>> is selected.
>>>>>>> So I think we can remove _jvmti_force_gc from is_user_requested_gc() and add
>>>>>>> _dcmd_gc_run
>>>>>>> to it.
>>>>>>>
>>>>>>>> A "grep" for _java_lang_system_gc should yield more places where updates may
>>>>>>>> be necessary.
>>>>>>>
>>>>>>> We can use GCCause::is_user_requested_gc() if the proposal in above is
>>>>>>> accepted.
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 2015/02/13 21:33, Mikael Gerdin wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> On 2015-02-11 15:02, Yasumasa Suenaga wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> I've committed JDK-8068589 to add new GCCause - Diagnostic Command.
>>>>>>>>> However, it has been backouted because test is failed [1] and it is not
>>>>>>>>> considered
>>>>>>>>> about concurrent GC: -XX:+ExplicitGCInvokesConcurrent [2].
>>>>>>>>>
>>>>>>>>> I've created patch for this enhancement.
>>>>>>>>> Could you review it?
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.00/
>>>>>>>>
>>>>>>>> I'd prefer if you could add a GCCause::is_system_gc_equivalent() which
>>>>>>>> returns true for some set of GCCause enum values, such as
>>>>>>>> _java_lang_system_gc and _dcmd_gc_run
>>>>>>>>
>>>>>>>> Given that the documentation of the GC.run command is:
>>>>>>>> "GC.run
>>>>>>>> Call java.lang.System.gc().
>>>>>>>>
>>>>>>>> Impact: Medium: Depends on Java heap size and content.
>>>>>>>>
>>>>>>>> Syntax: GC.run"
>>>>>>>>
>>>>>>>> I interpret the documentation that the GC is supposed to be (for all intents
>>>>>>>> and purposes) equivalent to the application invoking System.gc().
>>>>>>>>
>>>>>>>> This would also require updates to other places where we refer to the
>>>>>>>> _java_lang_system_gc GCCause, such as UseAdaptiveSizePolicyWithSystemGC
>>>>>>>>
>>>>>>>> A "grep" for _java_lang_system_gc should yield more places where updates may
>>>>>>>> be necessary.
>>>>>>>>
>>>>>>>> /Mikael
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'm jdk9 committer, but I'm not employee at Oracle.
>>>>>>>>> So I need a Sponsor.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [1]
>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011957.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [2]
>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011962.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
[prev in list] [next in list] [prev in thread] [next in thread]