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

[Jesper Wilhelmsson]

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