Hi Serguei,
On 2020/07/01 8:24, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
Thank you for separating your initial webrev.
I'll do a full review after you address comments from David and Robbin as I'm
stepping on the same ground.
Just a quick comment now.
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.00/src/hotspot/share/prims/jvmtiEnv.cpp.udiff.html
I've already asked in prev. round to make this renaming: target_javathread =>
java_thread
The identifier java_thread is normally used in the jvmtiEnv.cpp functions.
The target_javathread sounds very unusual.
Sorry, I've fixed it. I will update webrev.
I do not like much the introduction ofthe GetSingleStackTraceClosure.
It feels like it can be done in a more elegant way.
From the other hand, it is not that bad. :-)
I thought to use handshake on VMThread (!= direct handshake) for
GetThreadListStackTraces() to be simplify implementation.
However it would be queued as VM op (of course STW would not happen), so I
introduced GetSingleStackTraceClosure.
Thanks,
Yasumasa
Thanks,
Serguei
On 6/29/20 17:05, Yasumasa Suenaga wrote:
Hi David, Serguei,
I updated webrev for 8242428. Could you review again?
This change migrate to use direct handshake for GetStackTrace() and
GetThreadListStackTraces() (when thread_count == 1).
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.01/
VM_GetThreadListStackTrace (for GetThreadListStackTraces) and
VM_GetAllStackTraces (for GetAllStackTraces) have inherited
VM_GetMultipleStackTraces VM operation which provides the feature to generate
jvmtiStackInfo. I modified VM_GetMultipleStackTraces to a normal C++ class to
share with HandshakeClosure for GetThreadListStackTraces
(GetSingleStackTraceClosure).
Also I added new testcases which test GetThreadListStackTraces() with
thread_count == 1 and with all threads.
This change has been tested in serviceability/jvmti serviceability/jdwp
vmTestbase/nsk/jvmti vmTestbase/nsk/jdi vmTestbase/nsk/jdwp.
Thanks,
Yasumasa
On 2020/06/24 15:50, Yasumasa Suenaga wrote:
Hi all,
Please review this change:
JBS: https://bugs.openjdk.java.net/browse/JDK-8242428
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.00/
This change replace following VM operations to direct handshake.
- VM_GetFrameCount (GetFrameCount())
- VM_GetFrameLocation (GetFrameLocation())
- VM_GetThreadListStackTraces (GetThreadListStackTrace())
- VM_GetCurrentLocation
GetThreadListStackTrace() uses direct handshake if thread count == 1. In other
case (thread count > 1), it would be performed as VM operation
(VM_GetThreadListStackTraces).
Caller of VM_GetCurrentLocation (JvmtiEnvThreadState::reset_current_location())
might be called at safepoint. So I added safepoint check in its caller.
This change has been tested in serviceability/jvmti serviceability/jdwp
vmTestbase/nsk/jvmti vmTestbase/nsk/jdi vmTestbase/ns
k/jdwp.
Also I tested it on submit repo, then it has execution error
(mach5-one-ysuenaga-JDK-8242428-20200624-0054-12034717) due to dependency
error. So I think it does not occur by this change.
Thanks,
Yasumasa
