<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Thanks Serguei!<br>
Coleen<br>
<br>
<div class="moz-cite-prefix">On 12/5/19 4:24 PM,
<a class="moz-txt-link-abbreviated" href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:5b865d09-2ec4-8378-b438-264279a9a6fd@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<div class="moz-cite-prefix">Got it, thanks!<br>
Serguei<br>
<br>
<br>
On 12/5/19 11:15, <a class="moz-txt-link-abbreviated"
href="mailto:coleen.phillimore@oracle.com"
moz-do-not-send="true">coleen.phillimore@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:66e249d2-ea32-db69-5e1f-1a31fb19ecd5@oracle.com"> <br>
<br>
<div class="moz-cite-prefix">On 12/5/19 1:41 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:b962601a-3bbe-3a9f-c96b-c35edcdb9c32@oracle.com">
<div class="moz-cite-prefix">On 12/5/19 10:36, <a
class="moz-txt-link-abbreviated"
href="mailto:coleen.phillimore@oracle.com"
moz-do-not-send="true">coleen.phillimore@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:a675ed22-91eb-4571-2578-8c5cd774d74f@oracle.com">
<br>
<br>
<div class="moz-cite-prefix">On 12/5/19 11:00 AM, <a
class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:2385071e-6fd6-3da4-9ab3-93e950f69f77@oracle.com">
<div class="moz-cite-prefix">Hi Collen,<br>
<br>
Thank you for making this update!<br>
It looks good to me.<br>
<br>
One nit:<br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.03/webrev/test/hotspot/jtreg/serviceability/jvmti/CompiledMethodLoad/libCompiledZombie.cpp.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.03/webrev/test/hotspot/jtreg/serviceability/jvmti/CompiledMethodLoad/libCompiledZombie.cpp.html</a><br>
<br>
46 // Continuously generate CompiledMethodLoad events
for all currently compiled methods<br>
47 void JNICALL GenerateEventsThread(jvmtiEnv* jvmti,
JNIEnv* jni, void* arg) {<br>
48
jvmti->SetEventNotificationMode(JVMTI_ENABLE,
JVMTI_EVENT_COMPILED_METHOD_LOAD, NULL);<br>
49 int count = 0;<br>
50 <br>
51 while (true) {<br>
52 events = 0;<br>
53
jvmti->GenerateEvents(JVMTI_EVENT_COMPILED_METHOD_LOAD);<br>
54 if (events != 0 && ++count == 200)
{<br>
55 printf("Generated %d events\n",
events);<br>
56 count = 0;<br>
57 }<br>
58 }<br>
59 }<br>
<br>
The above can be simplified a little bit:<br>
if (events % 200 == 199) {<br>
printf("Generated %d events\n", events);<br>
}<br>
<br>
Then this line is not needed too:<br>
49 int count = 0;<br>
<br>
</div>
</blockquote>
<br>
I answered this too fast. There are two conditions where I
want this to not print. First is where events == 0 and the
other for every 200 events that are non-zero.<br>
<br>
I could use if (events != 0 && count++ % 200), but I
thought what I had makes more sense and I don't have to
worry about when ++ happens.<br>
</blockquote>
<br>
Then you could replace it with:<br>
if (events % 200 == 0) {<br>
</blockquote>
<br>
But that would still print when events == 0, which I don't
want. If I print them all for the little test case, it's ok,
but when I run this with Swingset2, it's too much output. I
only want to see a few lines for this:<br>
<br>
----------System.out:(3/113)----------<br>
Test passes if it doesn't crash while posting compiled method
events.<br>
Generated 285 events<br>
Generated 1002 events<br>
----------System.err:(1/15)----------<br>
<br>
The count is the number of times through the GenerateEvents
loop, which resets events to zero each time, then prints the
number of events for every 200 times through the GenerateEvents
loop. So I need both count and events.<br>
<br>
Coleen<br>
<blockquote type="cite"
cite="mid:b962601a-3bbe-3a9f-c96b-c35edcdb9c32@oracle.com"> <br>
But it is up to you. :)<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:a675ed22-91eb-4571-2578-8c5cd774d74f@oracle.com">
<br>
Thanks,<br>
Coleen<br>
<br>
<blockquote type="cite"
cite="mid:2385071e-6fd6-3da4-9ab3-93e950f69f77@oracle.com">
<div class="moz-cite-prefix"> <br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 12/5/19 04:08, <a class="moz-txt-link-abbreviated"
href="mailto:coleen.phillimore@oracle.com"
moz-do-not-send="true">coleen.phillimore@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:9173a527-bbdc-e04d-3981-f1cc0efbdca0@oracle.com">
<br>
Thanks Dan. I moved the field. For some reason I
thought that class did more/different things than hold
per-thread information.<br>
<br>
I've retested this version with tiers 2-6.<br>
<br>
incr webrev at <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.03.incr/webrev"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.03.incr/webrev</a><br>
full webrev at <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.03/webrev"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.03/webrev</a><br>
<br>
Thanks to Serguei for offline discussion.<br>
<br>
Coleen<br>
<br>
<div class="moz-cite-prefix">On 12/4/19 7:40 PM, Daniel
D. Daugherty wrote:<br>
</div>
<blockquote type="cite"
cite="mid:7d5393cb-3783-9e89-6de8-f3d2f87b017d@oracle.com">
<tt>Generally speaking, JVM/TI related things should
be in JvmtiThreadState<br>
instead of directly in the Thread class. That way
the extra space is only<br>
consumed when JVM/TI is in use and only when a
Thread does something that<br>
requires a JvmtiThreadState to be created.<br>
<br>
Please reconsider moving _jvmti_event_queue.<br>
<br>
Dan<br>
<br>
</tt><br>
<div class="moz-cite-prefix">On 12/4/19 6:06 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:coleen.phillimore@oracle.com"
moz-do-not-send="true">coleen.phillimore@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:3d6ace12-ef89-64e4-02fc-3fe8806772e1@oracle.com">
<br>
Hi Serguei,<br>
<br>
<div class="moz-cite-prefix">On 12/4/19 5:15 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:008cc38c-a407-3acf-3769-7b8b6f47fcb3@oracle.com">
<div class="moz-cite-prefix">Hi Collen, (no
problem)<br>
<br>
It looks good in general.<br>
Thank you a lot for sorting this out!<br>
<br>
Just a couple of comments.<br>
<br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev/src/hotspot/share/runtime/thread.hpp.frames.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev/src/hotspot/share/runtime/thread.hpp.frames.html</a><br>
<pre><span class="new">1993 protected:</span>
<span class="new">1994 // Jvmti Events that cannot be posted in their current context.</span>
<span class="new">1995 // ServiceThread uses this to collect deferred events from NonJava threads</span>
<span class="new">1996 // that cannot post events.</span>
<span class="new">1997 JvmtiDeferredEventQueue* _jvmti_event_queue;</span>
</pre>
<br>
As David I also have a concern about footprint
of having the <span class="new">_jvmti_event_queue</span>
field in the Thread class.<br>
I'm thinking if it'd be better to move this
field into the JvmtiThreadState class.<br>
Please, see jvmti_thread_state() and
JvmtiThreadState::state_for(JavaThread *thread).<br>
</div>
</blockquote>
<br>
The reason I have it directly in JavaThread is so
that the GC oops_do and nmethods_do code can find it
easily. I like your idea of hiding it in jvmti but
this doesn't seem good to have this code know about
jvmtiThreadState, which seems to be a queue of Jvmti
states. I also don't want to have jvmtiThreadState
to have to add an oops_do() or nmethods_do() either.<br>
<br>
<blockquote type="cite"
cite="mid:008cc38c-a407-3acf-3769-7b8b6f47fcb3@oracle.com">
<div class="moz-cite-prefix"> <br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.frames.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.frames.html</a><br>
<pre><span class="new"> 973 void JvmtiDeferredEvent::post(JvmtiEnv* env) {</span>
<span class="new"> 974 assert(_type == TYPE_COMPILED_METHOD_LOAD, "only user of this method");</span>
<span class="new"> 975 nmethod* nm = _event_data.compiled_method_load;</span>
<span class="new"> 976 JvmtiExport::post_compiled_method_load(env, nm);</span>
<span class="new"> 977 }</span></pre>
<br>
The <span class="new"> JvmtiDeferredEvent::post
name looks too generic as it posts compiled
load events only.<br>
Do you consider this function extended in the
future to support more event types?<br>
<br>
</span></div>
</blockquote>
<br>
I don't envision an extension for this function but
I do for JvmtiDeferredEventQueue::post(). I have a
small enhancement that would handoff the entire
queue to the ServiceThread and have it call post()
to post all the events rather than one at a time.<br>
<br>
So I'll rename this one
post_compiled_method_load_event() and leave the
other post() as is for now.<br>
<br>
open webrev at <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.02.incr/webrev"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.02.incr/webrev</a><br>
<br>
Thanks,<br>
Coleen<br>
<br>
<br>
<blockquote type="cite"
cite="mid:008cc38c-a407-3acf-3769-7b8b6f47fcb3@oracle.com">
<div class="moz-cite-prefix"><span class="new"> <br>
Thanks,<br>
Serguei<br>
</span> <br>
<br>
On 11/26/19 06:22, <a
class="moz-txt-link-abbreviated"
href="mailto:coleen.phillimore@oracle.com"
moz-do-not-send="true">coleen.phillimore@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:886380dd-fa13-94e5-ba1d-fc4678a5f90c@oracle.com">Summary:
Add local deferred event list to thread to post
events outside CodeCache_lock. <br>
<br>
This patch builds on the patch for JDK-8173361.
With this patch, I made the
JvmtiDeferredEventQueue an instance class (not
AllStatic) and have one per thread. The
CodeBlob event that used to drop the
CodeCache_lock and raced with the sweeper
thread, adds the events it wants to post to its
thread local list, and processes it outside the
lock. The list is walked in GC and by the
sweeper to keep the nmethods from being unloaded
and zombied, respectively. <br>
<br>
Also, the jmethod_id field in nmethod was only
used as a boolean so don't create a jmethod_id
until needed for post_compiled_method_unload. <br>
<br>
Ran hs tier1-8 on linux-x64-debug and the stress
test that crashed in the original bug report. <br>
<br>
open webrev at <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev"
moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev</a>
<br>
bug link <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8212160"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212160</a>
<br>
<br>
Thanks, <br>
Coleen <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>