On 2020/07/11 22:50, David Holmes wrote:
On 11/07/2020 11:52 am, Yasumasa Suenaga wrote:
Thanks Dan!
David, Serguei, are you ok to this change?
Yes it seems fine.
Thanks David!
In relation to an earlier comment:
> I replaced %p to %lx, and also cast values to unsigned long
Never use l
On 11/07/2020 11:52 am, Yasumasa Suenaga wrote:
Thanks Dan!
David, Serguei, are you ok to this change?
Yes it seems fine.
In relation to an earlier comment:
> I replaced %p to %lx, and also cast values to unsigned long
Never use long or unsigned long as they can be different sizes on
diffe
Thanks Serguei!
Yasumasa
On 2020/07/11 14:51, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
I'm okay with the update.
Thanks,
Serguei
On 7/10/20 18:52, Yasumasa Suenaga wrote:
Thanks Dan!
David, Serguei, are you ok to this change?
Yasumasa
On 2020/07/11 7:00, Daniel D. Daugherty wro
Hi Yasumasa,
I'm okay with the update.
Thanks,
Serguei
On 7/10/20 18:52, Yasumasa Suenaga wrote:
Thanks Dan!
David, Serguei, are you ok to this change?
Yasumasa
On 2020/07/11 7:00, Daniel D. Daugherty wrote:
On 7/10/20 3:29 AM, Yasumasa Suenaga wrote:
Thanks Patricio!
I uploaded new w
Thanks Dan!
David, Serguei, are you ok to this change?
Yasumasa
On 2020/07/11 7:00, Daniel D. Daugherty wrote:
On 7/10/20 3:29 AM, Yasumasa Suenaga wrote:
Thanks Patricio!
I uploaded new webrev. Could you review again?
It passed all tests on submit repo, and passed jtreg tests about JVMTI.
On 7/10/20 3:29 AM, Yasumasa Suenaga wrote:
Thanks Patricio!
I uploaded new webrev. Could you review again?
It passed all tests on submit repo, and passed jtreg tests about JVMTI.
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.10/
Diff from webrev.09:
http://cr.openjdk.java.net/~y
Thanks Patricio!
I uploaded new webrev. Could you review again?
It passed all tests on submit repo, and passed jtreg tests about JVMTI.
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.10/
Diff from webrev.09:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.10/incremental/
Hi Yasumasa,
On 7/10/20 1:58 AM, Yasumasa Suenaga wrote:
Hi Patricio,
Thanks for your advice!
I've fixed testcase as [1], but I still see an error in
validate-headers-linux-x64-build-1 on submit repo.
What does it mean? Can you share details?
mach5-one-ysuenaga-JDK-8242428-20200710-0339-12
Hi Patricio,
Thanks for your advice!
I've fixed testcase as [1], but I still see an error in
validate-headers-linux-x64-build-1 on submit repo.
What does it mean? Can you share details?
mach5-one-ysuenaga-JDK-8242428-20200710-0339-12529134
Of course this change can be built on my Linux box (
On 7/9/20 12:00 PM, Patricio Chilano wrote:
Hi Yasumasa,
On 7/9/20 9:30 AM, Yasumasa Suenaga wrote:
On 2020/07/09 17:58, David Holmes wrote:
Hi Yasumasa,
On 9/07/2020 10:25 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysu
Hi Yasumasa,
On 7/9/20 9:30 AM, Yasumasa Suenaga wrote:
On 2020/07/09 17:58, David Holmes wrote:
Hi Yasumasa,
On 9/07/2020 10:25 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/
Diff from pr
On 2020/07/09 17:58, David Holmes wrote:
Hi Yasumasa,
On 9/07/2020 10:25 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/
Diff from previous webrev:
http://hg.openjdk.java.net/jdk/submit/rev/
Hi Yasumasa,
On 9/07/2020 10:25 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/
Diff from previous webrev:
http://hg.openjdk.java.net/jdk/submit/rev/5d167adf8524
I saw similar build errors
On 2020/07/09 9:25, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/
Diff from previous webrev:
http://hg.openjdk.java.net/jdk/submit/rev/5d167adf8524
Submit repo reported some build errors on ma
Hi Dan,
Thanks for your comment!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.09/
Diff from previous webrev:
http://hg.openjdk.java.net/jdk/submit/rev/5d167adf8524
I saw similar build errors in libOneGetThreadListStackTraces.cpp on Windows.
This webrev fix
> http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.08/
src/hotspot/share/prims/jvmtiEnv.cpp
No comments.
src/hotspot/share/prims/jvmtiEnvBase.cpp
L1159: Thread *current_thread = Thread::current();
Please add "#ifdef ASSERT" above and "#endif" below since
current_
Thanks David!
Yasumasa
On 2020/07/08 21:29, David Holmes wrote:
On 8/07/2020 6:04 pm, Yasumasa Suenaga wrote:
Hi David,
On 2020/07/08 15:27, David Holmes wrote:
Hi Yasumasa,
On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote:
Hi David, Serguei,
Serguei, thank you for replying even though you ar
On 8/07/2020 6:04 pm, Yasumasa Suenaga wrote:
Hi David,
On 2020/07/08 15:27, David Holmes wrote:
Hi Yasumasa,
On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote:
Hi David, Serguei,
Serguei, thank you for replying even though you are on vacaiton!
I uploaded new webrev:
http://cr.openjdk.java.n
Hi David,
On 2020/07/08 15:27, David Holmes wrote:
Hi Yasumasa,
On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote:
Hi David, Serguei,
Serguei, thank you for replying even though you are on vacaiton!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.07/
Diff fro
Hi Yasumasa,
On 7/07/2020 6:54 pm, Yasumasa Suenaga wrote:
Hi David, Serguei,
Serguei, thank you for replying even though you are on vacaiton!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.07/
Diff from previous webrev:
http://hg.openjdk.java.net/jdk/su
Hi David, Serguei,
Serguei, thank you for replying even though you are on vacaiton!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.07/
Diff from previous webrev:
http://hg.openjdk.java.net/jdk/submit/rev/77243b1dcbfe
c'tor of GetSingleStackTraceClosure has
Hi Yasumasa,
Thank you for this update - the test looks much better and is readable now.
I'm on vacation this week but will try to look at your fixes a little
bit more.
As I understand you are going to post one more update.
Thanks,
Serguei
On 7/6/20 06:29, Yasumasa Suenaga wrote:
Hi Serguei
On 7/07/2020 2:57 pm, Yasumasa Suenaga wrote:
Hi David,
On 2020/07/07 11:31, David Holmes wrote:
Hi Yasumasa,
Hard to keep up with the changes - especially without incremental
webrevs.
Sorry, I will upload diff from previous webrev in the next.
If GetSingleStackTraceClosure also took the
Hi David,
On 2020/07/07 11:31, David Holmes wrote:
Hi Yasumasa,
Hard to keep up with the changes - especially without incremental webrevs.
Sorry, I will upload diff from previous webrev in the next.
If GetSingleStackTraceClosure also took the jthread as a constructor arg, then
you wouldn'
Hi Yasumasa,
Hard to keep up with the changes - especially without incremental webrevs.
If GetSingleStackTraceClosure also took the jthread as a constructor
arg, then you wouldn't need to recreate a JNI local handle when calling
_collector.fill_frames. It's a small simplification and not essen
Hi Serguei,
Thanks for your comment!
I think C++ is more simple to implement the test agent as you said.
So I implement it in C++ in new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.06/
Also I refactored libGetThreadListStackTraces.cpp, and I've kep
Hi Yasumasa,
Thank you for the update.
I think, a pending exception after IsSameObject needs to be
checked.
The checkStackInfo() needs one more refactoring as I've already
suggested.
The body of the loop at L68-L78 should be converted to
Hi Serguei,
Thanks for your comment!
I refactored testcase. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.05/
It would check Java exception after IsSameObject() call. Does it need?
Any exceptions are not described in JNI document[1], and JNI implementation
(
Hi Yasumasa,
Okay, thanks.
Then I'm okay to keep the GetSingleStackTraceClosure.
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.04/test/hotspot/jtreg/serviceability/jvmti/GetThreadListStackTraces/libGetThreadListStackTraces.c.html
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webre
Hi Serguei,
I'm not an Oracle employee, so I cannot know real request(s) from your
customers.
However JDK-8201641 says Dynatrace has requested this enhancement.
BTW I haven't heared any request from my customers about this.
Thanks,
Yasumasa
On 2020/07/04 4:32, serguei.spit...@oracle.com wr
Hi Yasumasa,
This difference is not that big to care about.
I feel this is really rare case and so, does not worth these complications.
Do we have a real request from customers to optimize it?
Thanks,
Serguei
On 7/3/20 01:16, Yasumasa Suenaga wrote:
Hi Serguei,
Generally I agree with you, bu
Hi Serguei,
Generally I agree with you, but I have concern about the difference of the
result of GetStackTrace() and GetThreadListStackTraces().
GetStackTrace: jvmtiFrameInfo
GetThreadListStackTraces: jvmtiStackInfo
jvmtiStackInfo contains thread state, and it is ensured it is the state of
Hi Yasumasa,
After some thinking I've concluded that I do not like this
optimization
of the GetThreadListStackTraces with
GetSingleStackTraceClosure.
We may need more opinions on this but these are my points:
- it adds some compl
Hi Dan, David,
I uploaded new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.04/
OneGetThreadListStackTraces.java in this webrev would wait until thread state is
transited to "waiting" with spin wait.
CountDownLatch::await call as Dan pointed is fixed
On 3/07/2020 2:27 pm, Yasumasa Suenaga wrote:
On 2020/07/03 12:24, Daniel D. Daugherty wrote:
On 7/2/20 10:50 PM, David Holmes wrote:
Sorry I'm responding here without seeing latest webrev but there is
enough context I think ...
On 3/07/2020 9:14 am, Yasumasa Suenaga wrote:
Hi Dan,
Thank
On 3/07/2020 1:24 pm, Daniel D. Daugherty wrote:
On 7/2/20 10:50 PM, David Holmes wrote:
Sorry I'm responding here without seeing latest webrev but there is
enough context I think ...
On 3/07/2020 9:14 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
On 2020/07/03 7:16, Daniel D
On 2020/07/03 12:24, Daniel D. Daugherty wrote:
On 7/2/20 10:50 PM, David Holmes wrote:
Sorry I'm responding here without seeing latest webrev but there is enough
context I think ...
On 3/07/2020 9:14 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
On 2020/07/03 7:16, Daniel D.
On 7/2/20 10:50 PM, David Holmes wrote:
Sorry I'm responding here without seeing latest webrev but there is
enough context I think ...
On 3/07/2020 9:14 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
On 2020/07/03 7:16, Daniel D. Daugherty wrote:
On 7/2/20 5:19 AM, Yasumasa Su
Sorry I'm responding here without seeing latest webrev but there is
enough context I think ...
On 3/07/2020 9:14 am, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
On 2020/07/03 7:16, Daniel D. Daugherty wrote:
On 7/2/20 5:19 AM, Yasumasa Suenaga wrote:
Hi David,
I upload new web
On 7/2/20 7:14 PM, Yasumasa Suenaga wrote:
Hi Dan,
Thanks for your comment!
On 2020/07/03 7:16, Daniel D. Daugherty wrote:
On 7/2/20 5:19 AM, Yasumasa Suenaga wrote:
Hi David,
I upload new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.03/
src/hots
Hi Dan,
Thanks for your comment!
On 2020/07/03 7:16, Daniel D. Daugherty wrote:
On 7/2/20 5:19 AM, Yasumasa Suenaga wrote:
Hi David,
I upload new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.03/
src/hotspot/share/prims/jvmtiEnv.cpp
L1542:
On 7/2/20 5:19 AM, Yasumasa Suenaga wrote:
Hi David,
I upload new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.03/
src/hotspot/share/prims/jvmtiEnv.cpp
L1542: // Get stack trace with handshake
nit - please add a period at the end.
Hi David,
I upload new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.03/
On 2020/07/02 15:05, David Holmes wrote:
Hi Yasumasa,
On 1/07/2020 11:53 am, Yasumasa Suenaga wrote:
Hi,
I uploaded new webrev. Could review again?
http://cr.openjdk.java
Hi Yasumasa,
On 1/07/2020 11:53 am, Yasumasa Suenaga wrote:
Hi,
I uploaded new webrev. Could review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.02/
Updates look fine - thanks.
One minor nit:
1274 _collector.allocate_and_fill_stacks(1);
1275 _collector.set_resu
Hi,
I uploaded new webrev. Could review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.02/
src/hotspot/share/prims/jvmtiEnvBase.cpp
820 assert(SafepointSynchronize::is_at_safepoint() ||
821 java_thread->is_thread_fully_suspended(false, &debug_bits) ||
822
On 2020/07/01 8:54, David Holmes wrote:
On 1/07/2020 9:51 am, Yasumasa Suenaga wrote:
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
ste
On 1/07/2020 9:51 am, Yasumasa Suenaga wrote:
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
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/~ysuena
Hi Yasumasa,
On 1/07/2020 9:05 am, Yasumasa Suenaga wrote:
Hi David,
1271 ResourceMark rm;
IIUC at this point the _calling_thread is the current thread, so we
can use:
ResourceMark rm(_calling_thread);
If so, we can call make_local() in L1272 without JavaThread (or we can
pass
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/we
Hi David,
1271 ResourceMark rm;
IIUC at this point the _calling_thread is the current thread, so we can use:
ResourceMark rm(_calling_thread);
If so, we can call make_local() in L1272 without JavaThread (or we can pass
current thread to make_local()). Is it right?
```
1271 Res
On 1/07/2020 12:17 am, Yasumasa Suenaga wrote:
Hi David,
Thank you for reviewing! I will update new webrev tomorrow.
466 class MultipleStackTracesCollector : public StackObj {
498 class VM_GetAllStackTraces : public VM_Operation {
499 private:
500 JavaThread *_calling_thread;
501
Hi David,
Thank you for reviewing! I will update new webrev tomorrow.
466 class MultipleStackTracesCollector : public StackObj {
498 class VM_GetAllStackTraces : public VM_Operation {
499 private:
500 JavaThread *_calling_thread;
501 jint _final_thread_count;
502 MultipleStackT
Hi Robbin,
On 30/06/2020 11:03 pm, Robbin Ehn wrote:
Hi Yasumasa,
On 2020-06-30 14:35, Yasumasa Suenaga wrote:
Hi Robbin,
We decided to separate thread operation and frame operation.
I've posted review request for thread operation. Could you review it?
Yes I know but I'm soon off for vacati
Hi Yasumasa,
On 30/06/2020 10:05 am, 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/~ysuena
Hi Yasumasa,
On 2020-06-30 14:35, Yasumasa Suenaga wrote:
Hi Robbin,
We decided to separate thread operation and frame operation.
I've posted review request for thread operation. Could you review it?
Yes I know but I'm soon off for vacation and you have not sent them all
out and due to the na
Hi Robbin,
We decided to separate thread operation and frame operation.
I've posted review request for thread operation. Could you review it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8242428/webrev.01/
We can share HandshakeClosure for GetStackTrace() to GetFrameLocation() as you
said.
Howev
Hi Yasumasa,
Thanks for your effort doing this.
#1
GetFrameLocation
GetStackTrace
GetCurrentLocation (need to add BCI)
Should use exactly the same code since a stack trace with max_count = 1
and start_depth = depth/0 is the frame location and jvmtiFrameInfo
contain the correct information (+ ad
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 GetThr
Hi Serguei,
Thanks for your comment!
I will fix them after JDK-8248379.
Yasumasa
On 2020/06/26 13:48, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
I agree with the approach to separate this into different bugs.
At least, it would be nice to separate the stack trace functions.
It will help
Hi Yasumasa,
I agree with the approach to separate this into different bugs.
At least, it would be nice to separate the stack trace functions.
It will help to better focus on each fix and improve review
quality.
I'd wait for new webrevs from yo
Hi David,
We can get the result from result() in their Closures.
JVMTI_ERROR_THREAD_NOT_ALIVE is set by default in
GetCurrentContendedMonitorClosure, so we can get this error if the handshake is
not completed.
Should I check result of execute_direct() even if that?
(Of course, we should fix G
On 26/06/2020 11:31 am, Yasumasa Suenaga wrote:
On 2020/06/25 21:48, David Holmes wrote:
You are not checking the return value of Handshake::execute_direct
and so are missing the possibility that the target thread has
terminated before you got to do the operation on it. It isn't clear
to me
Hi David,
On 2020/06/25 21:48, David Holmes wrote:
Hi Yasumasa,
On 25/06/2020 6:24 pm, Yasumasa Suenaga wrote:
Hi David,
Thanks for your comment!
On 2020/06/25 14:17, David Holmes wrote:
Hi Yasumasa,
Thanks for tackling this. I've had an initial look at it and have a few
concerns.
On 24/
Hi Yasumasa,
On 25/06/2020 6:24 pm, Yasumasa Suenaga wrote:
Hi David,
Thanks for your comment!
On 2020/06/25 14:17, David Holmes wrote:
Hi Yasumasa,
Thanks for tackling this. I've had an initial look at it and have a
few concerns.
On 24/06/2020 4:50 pm, Yasumasa Suenaga wrote:
Hi all,
P
Hi David,
Thanks for your comment!
On 2020/06/25 14:17, David Holmes wrote:
Hi Yasumasa,
Thanks for tackling this. I've had an initial look at it and have a few
concerns.
On 24/06/2020 4:50 pm, Yasumasa Suenaga wrote:
Hi all,
Please review this change:
JBS: https://bugs.openjdk.java.ne
Hi Yasumasa,
Thanks for tackling this. I've had an initial look at it and have a few
concerns.
On 24/06/2020 4:50 pm, 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-82424
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 (GetFra
68 matches
Mail list logo
