PING: RFR: JDK-8072913: [REDO] GCCause should distinguish jcmd GC.run from System.gc()

[Jesper Wilhelmsson]

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
>>>>>>>>
>>>>>>>>
>>>>>>>>