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

[Yasumasa Suenaga]

I need more reviewer.
Could you review?

http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.02/

Thanks,

Yasumasa
2015/05/28 23:02 "Yasumasa Suenaga" <yasuenag at gmail.com>:

> 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150603/dbf41e28/attachment.htm>