[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: PING: RFR: JDK-8072913: [REDO] GCCause should distinguish jcmd GC.run from System.gc()
From: Yasumasa Suenaga <yasuenag () gmail ! com>
Date: 2015-06-02 15:17:55
Message-ID: CAGFVN2DEL4BXHzYAtHkLxhMspsL32PvdNk5O6B+2Bwm+nEeu-g () mail ! gmail ! com
[Download RAW message or body]
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@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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
[Attachment #3 (text/html)]
<p dir="ltr">I need more reviewer.<br>
Could you review?</p>
<p dir="ltr"><a href="http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.02/">http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.02/</a><br></p>
<p dir="ltr">Thanks,</p>
<p dir="ltr">Yasumasa<br></p>
<div class="gmail_quote">2015/05/28 23:02 "Yasumasa Suenaga" <<a \
href="mailto:yasuenag@gmail.com">yasuenag@gmail.com</a>>:<br \
type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I'll \
sponsor it.<br> </blockquote>
<br>
Thank you Jesper!<br>
I will send a chengeset after reviewing.<br>
<br>
<br>
Yasumasa<br>
<br>
<br>
On 2015/05/28 22:47, Jesper Wilhelmsson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Looks good!<br>
I'll sponsor it.<br>
/Jesper<br>
<br>
Yasumasa Suenaga skrev den 28/5/15 05:31:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> I've uploaded new webrev:<br>
<a href="http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.02/" \
target="_blank">http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.02/</a><br> \
<br> I need a Sponsor and more reviewer.<br>
Please review it.<br>
<br>
<br>
Thanks,<br>
<br>
Yasumasa<br>
<br>
<br>
On 2015/05/28 8:52, Yasumasa Suenaga wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi Jesper,<br>
<br>
Thank you for your comment.<br>
I will fix it.<br>
<br>
<br>
Thanks,<br>
<br>
Yasumasa<br>
<br>
<br>
On 2015/05/28 5:14, Jesper Wilhelmsson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi,<br>
<br>
I like that you removed _jvmti_force_gc from is_user_requested_gc() and used<br>
this method throughout. It is cleaner and is_user_requested_gc() makes more<br>
sense now.<br>
<br>
In vmCMSOperations.cpp I think the comment should say GCCause::_dcmd_gc_run.<br>
<br>
Besides that minor comment, looks good!<br>
<br>
Thanks,<br>
/Jesper<br>
<br>
<br>
Yasumasa Suenaga skrev den 20/4/15 15:53:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi all,<br>
<br>
I've uploaded webrev for this enhancement.<br>
Could you review it?<br>
<br>
<a href="http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/" \
target="_blank">http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/</a><br> \
<br> <br>
Thanks,<br>
<br>
Yasumasa<br>
<br>
<br>
On 2015/03/11 22:13, Yasumasa Suenaga wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi all,<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> So I think we can remove _jvmti_force_gc from \
is_user_requested_gc() and add<br> _dcmd_gc_run<br>
to it.<br>
</blockquote>
<br>
I've uploaded new webrev, and I've applied it to new patch.<br>
Could you review it?<br>
<br>
<a href="http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/" \
target="_blank">http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/</a><br> \
<br> I also updated jtreg testcase.<br>
It works fine in my environment.<br>
<br>
<br>
Thanks,<br>
<br>
Yasumasa<br>
<br>
<br>
On 2015/02/14 22:10, Yasumasa Suenaga wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi Mikael,<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> I'd prefer if you could add a \
GCCause::is_system_gc_equivalent() which<br> returns true for some set of GCCause \
enum values, such as<br> _java_lang_system_gc and _dcmd_gc_run<br>
</blockquote>
<br>
Can I add _dcmd_gc_run to GCCause::is_user_requested_gc() ?<br>
This function is used with GCCause::is_serviceability_requested_gc() .<br>
CMSCollector::is_external_interruption() and<br>
AdaptiveSizePolicy::check_gc_overhead_limit()<br>
<br>
is_user_requested_gc() and is_serviceability_requested_gc() checkes<br>
_jvmti_force_gc<br>
is selected.<br>
So I think we can remove _jvmti_force_gc from is_user_requested_gc() and add<br>
_dcmd_gc_run<br>
to it.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> A "grep" for _java_lang_system_gc should yield \
more places where updates may<br> be necessary.<br>
</blockquote>
<br>
We can use GCCause::is_user_requested_gc() if the proposal in above is<br>
accepted.<br>
<br>
<br>
Thanks<br>
<br>
Yasumasa<br>
<br>
<br>
<br>
On 2015/02/13 21:33, Mikael Gerdin wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi Yasumasa,<br>
<br>
On 2015-02-11 15:02, Yasumasa Suenaga wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Hi all,<br>
<br>
I've committed JDK-8068589 to add new GCCause - Diagnostic Command.<br>
However, it has been backouted because test is failed [1] and it is not<br>
considered<br>
about concurrent GC: -XX:+ExplicitGCInvokesConcurrent [2].<br>
<br>
I've created patch for this enhancement.<br>
Could you review it?<br>
<br>
<a href="http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.00/" \
target="_blank">http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.00/</a><br> \
</blockquote> <br>
I'd prefer if you could add a GCCause::is_system_gc_equivalent() which<br>
returns true for some set of GCCause enum values, such as<br>
_java_lang_system_gc and _dcmd_gc_run<br>
<br>
Given that the documentation of the GC.run command is:<br>
"GC.run<br>
Call java.lang.System.gc().<br>
<br>
Impact: Medium: Depends on Java heap size and content.<br>
<br>
Syntax: GC.run"<br>
<br>
I interpret the documentation that the GC is supposed to be (for all intents<br>
and purposes) equivalent to the application invoking System.gc().<br>
<br>
This would also require updates to other places where we refer to the<br>
_java_lang_system_gc GCCause, such as UseAdaptiveSizePolicyWithSystemGC<br>
<br>
A "grep" for _java_lang_system_gc should yield more places where updates \
may<br> be necessary.<br>
<br>
/Mikael<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <br>
<br>
I'm jdk9 committer, but I'm not employee at Oracle.<br>
So I need a Sponsor.<br>
<br>
<br>
Thanks,<br>
<br>
Yasumasa<br>
<br>
<br>
[1]<br>
<a href="http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011957.html" \
target="_blank">http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011957.html</a><br>
<br>
<br>
[2]<br>
<a href="http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011962.html" \
target="_blank">http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011962.html</a><br>
<br>
<br>
<br>
</blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></blockquote>
</blockquote></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic