Thank you, David!
Yasumasa, could you, please, send me a patch?
Thanks,
Serguei
On 10/19/17 22:05, David Holmes wrote:
Looks good. (Sorry for the delay.)
Thanks,
David
On 19/10/2017 11:43 PM, Yasumasa Suenaga wrote:
Sorry, I forgot the fix to use OrderAccess::load_acquire() in
PerfMemory::i
Looks good. (Sorry for the delay.)
Thanks,
David
On 19/10/2017 11:43 PM, Yasumasa Suenaga wrote:
Sorry, I forgot the fix to use OrderAccess::load_acquire() in
PerfMemory::is_initialized().
I fixed it in new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/we
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.09/
It looks good to me.
I've filed:
https://bugs.openjdk.java.net/browse/JDK-8189685
need PerfMemory class update and a volatile_static_field support in
VMStructs
Thanks!
Yasumasa
On 2017/10/20 2:49, serguei.spit...@oracle.co
Hi Yasumasa,
On 10/19/17 06:43, Yasumasa Suenaga wrote:
Sorry, I
forgot the fix to use OrderAccess::load_acquire() in
PerfMemory::is_initialized().
I fixed it in new webrev. Could you review again?
http://cr.open
Sorry, I forgot the fix to use OrderAccess::load_acquire() in
PerfMemory::is_initialized().
I fixed it in new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.09/
Yasumasa
On 2017/10/19 21:24, David Holmes wrote:
On 19/10/2017 9:44 PM, Yasumasa Suena
On 19/10/2017 9:44 PM, Yasumasa Suenaga wrote:
Hi,
I suggest we leave the volatile off for now and file a RFE to add
volatile_static_field support to VMStructs and update later.
Okay. David or Serguei, could you file it?
I'd suggest to fall back to your previous approach as synchronization
Hi,
I suggest we leave the volatile off for now and file a RFE to add
volatile_static_field support to VMStructs and update later.
Okay. David or Serguei, could you file it?
I'd suggest to fall back to your previous approach as synchronization was not
there
in the first place, and it is n
Hi Serguei, Yasumasa,
I suggest we leave the volatile off for now and file a RFE to add
volatile_static_field support to VMStructs and update later.
I don't think trying to introduce locking would be a good idea as it
would likely lead to deadlocks when a crash occurs. This could also be
inv
Hi Yasumasa,
I see the problem.
As it occurred making these variables volatile is non-trivial.
But thank you a lot for trying!
I'd suggest to fall back to your previous approach as synchronization
was not there
in the first place, and it is not a part of the original issue you are
trying to fi
Sorry, I have mistake.
But I cannot compile yet:
diff -r 3e7702cd3f19 src/hotspot/share/runtime/vmStructs.cpp
--- a/src/hotspot/share/runtime/vmStructs.cpp Thu Sep 07 15:40:20 2017 +0200
+++ b/src/hotspot/share/runtime/vmStructs.cpp Thu Oct 19 12:21:11 2017 +0900
@@ -578,7 +578,7 @@
sta
Hi Serguei,
Would the below work? :
578 static_field(PerfMemory, _initialized, volatile jint)
\
It'd be similar to this non-static case:
362 nonstatic_field(ConstantPoolCacheEntry, _f1,
volatile Metadata*)
On 10/18/17 06:51, Yasumasa Suenaga wrote:
Hi David, Serguei,
because as soon as we have checked is_usable() and abort happening in
another thread may have changed that by calling destroy.
This code is basically broken if we hit an abort path instead of a
normal VM shutdown.
Can we use Mut
On 10/18/17 05:34, David Holmes wrote:
Just to clarify ...
On 18/10/2017 10:28 PM, David Holmes wrote:
On 18/10/2017 8:26 PM, serguei.spit...@oracle.com wrote:
Hi David,
Thank you for jumping to this review and helping Yasumasa to sort it
out!
I've just discovered that this issue was already
On 10/18/17 05:28, David Holmes wrote:
On 18/10/2017 8:26 PM, serguei.spit...@oracle.com wrote:
Hi David,
Thank you for jumping to this review and helping Yasumasa to sort it
out!
I've just discovered that this issue was already on the table for
several months without a significant progress.
Hi David, Serguei,
because as soon as we have checked is_usable() and abort happening in another
thread may have changed that by calling destroy.
This code is basically broken if we hit an abort path instead of a normal VM
shutdown.
Can we use MutexLocker for initialize() and destroy() ?
Just to clarify ...
On 18/10/2017 10:28 PM, David Holmes wrote:
On 18/10/2017 8:26 PM, serguei.spit...@oracle.com wrote:
Hi David,
Thank you for jumping to this review and helping Yasumasa to sort it out!
I've just discovered that this issue was already on the table for
several months without
On 18/10/2017 8:26 PM, serguei.spit...@oracle.com wrote:
Hi David,
Thank you for jumping to this review and helping Yasumasa to sort it out!
I've just discovered that this issue was already on the table for
several months without a significant progress.
On 10/18/17 02:48, David Holmes wrote:
Hi David,
Thank you for jumping to this review and helping Yasumasa to sort
it out!
I've just discovered that this issue was already on the table for
several months without a significant progress.
On 10/18/17 02:48, David Holmes wrote:
On 18/10/2017 7:43 PM, Yasumasa Suenaga wrote:
Hi Serguei,
Should we use OrderAccess::load_acquire() to check _initialized and
_destroyed?
Yes for _initialized.
IMHO it do not need because initialize / destroy of PerfMemory seem not
to be on multi-threaded.
Initialization would normally b
Hi Serguei
On 18/10/2017 7:25 PM, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
Sorry for a quite late participation.
I looked at the previous webrevs and think that this one is much better.
Some concern is if we need any kind of synchronization here, e.g. CAS.
But it depends on the PerfMemo
Hi Serguei,
Should we use OrderAccess::load_acquire() to check _initialized and
_destroyed?
IMHO it do not need because initialize / destroy of PerfMemory seem not to
be on multi-threaded.
Thanks,
Yasumasa.
2017/10/18 午後6:25 "serguei.spit...@oracle.com" :
> Hi Yasumasa,
>
> Sorry for a quit
Hi Yasumasa,
Sorry for a quite late participation.
I looked at the previous webrevs and think that this one is much
better.
Some concern is if we need any kind of synchronization here, e.g.
CAS.
But it depends on the PerfMemory cla
Hi David,
Thank you for your comment.
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.07/
Serguei, please comment about this :-)
Yasumasa
2017-10-18 16:09 GMT+09:00 David Holmes :
> Hi Yasumasa,
>
> On 18/10/2017 4:34 PM, Yasumasa Suenaga wrote:
>>
>> Hi Dav
Hi Yasumasa,
On 18/10/2017 4:34 PM, Yasumasa Suenaga wrote:
Hi David,
I don't think we need the extra fields, just ensure the existing ones can't
be accessed (other than by the tools) after destroy is called.
I've added PerfMemory::is_useable() to check whether we can access to
PerfMemory.
I
Hi David,
> I don't think we need the extra fields, just ensure the existing ones can't
> be accessed (other than by the tools) after destroy is called.
I've added PerfMemory::is_useable() to check whether we can access to
PerfMemory.
I think this webrev prevent to access to PerfMemory after dest
On 18/10/2017 2:27 PM, Yasumasa Suenaga wrote:
Hi David,
2017-10-18 12:55 GMT+09:00 David Holmes :
On 18/10/2017 12:37 PM, Yasumasa Suenaga wrote:
Hi David,
With your changes you no longer null out _prologue so the assertion would
now not fail and we'd proceed to access the deleted memory r
Hi David,
2017-10-18 12:55 GMT+09:00 David Holmes :
> On 18/10/2017 12:37 PM, Yasumasa Suenaga wrote:
>>
>> Hi David,
>>
>>> With your changes you no longer null out _prologue so the assertion would
>>> now not fail and we'd proceed to access the deleted memory region!
>>
>>
>> On Linux, PerfMemor
On 18/10/2017 12:37 PM, Yasumasa Suenaga wrote:
Hi David,
With your changes you no longer null out _prologue so the assertion would
now not fail and we'd proceed to access the deleted memory region!
On Linux, PerfMemory::delete_memory_region() does not call munmap()
for PerfMemory.
Perhaps
Hi David,
> With your changes you no longer null out _prologue so the assertion would
> now not fail and we'd proceed to access the deleted memory region!
On Linux, PerfMemory::delete_memory_region() does not call munmap()
for PerfMemory.
> I'm unclear why you no longer clear all the fields set
Hi Yasumasa,
By chance we ran into this bug which I analysed yesterday:
https://bugs.openjdk.java.net/browse/JDK-8189390
We hit the assertion:
# Internal Error (/open/src/hotspot/share/runtime/perfMemory.cpp:216),
pid=17874, tid=17875
# assert(_prologue != __null) failed: called before ini
PING:
Could you review it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.05/
Thanks,
Yasumasa
On 2017/10/03 13:18, Yasumasa Suenaga wrote:
Hi all,
I added gtest unit test case for this change in new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.05/
Co
On 22/06/2016 9:40 AM, Yasumasa Suenaga wrote:
Hi all,
Can I add jdk9-fc-request label to JBS?
Bug fixes do not need approval only enhancements.
David
-
This changes has not been reviewed yet.
Thanks,
Yasumasa
2016/03/22 21:24 "Yasumasa Suenaga" mailto:yasue...@gmail.com>>:
PIN
Hi all,
Can I add jdk9-fc-request label to JBS?
This changes has not been reviewed yet.
Thanks,
Yasumasa
2016/03/22 21:24 "Yasumasa Suenaga" :
> PING: Could you review it?
>
> Yasumasa
> 2016/03/14 23:39 "Yasumasa Suenaga" :
>
>> Hi all,
>>
>> When I use `jhsdb jsnap` to get PerfCounter from co
PING: What do you think about this change?
Point of troubleshooting, I want to fix this issue.
Thanks,
Yasumasa
On 2016/05/25 20:58, Yasumasa Suenaga wrote:
Hi Dmitry,
I understand the problem, but I'm concerned of storing pointers to
invalid memory region just for coredump. Sorry!
The l
Hi Dmitry,
I understand the problem, but I'm concerned of storing pointers to
invalid memory region just for coredump. Sorry!
The latest webrev [1] holds pointers for PerfMemory in _saved_* fields.
These pointers are valid because they are not unmap'ed.
I think that it is a bug not to parse P
Dmitry,
The solution might be in another areas, e.g. print value of performance
counters to hs_err.log on crash.
If so, we have to use native debugger.
For Java developers and troubleshooters, I want to support this feature
with JDK tool.
If we encounter native stack overflow error, we canno
Hi Dmitry,
On 2016/05/06 21:22, Dmitry Samersoff wrote:
Yasumasa,
I understand the problem, but I'm concerned of storing pointers to
invalid memory region just for coredump. Sorry!
I do not think that it is NOT invalid memory in webrev.02 .
PerfMemory is mmap'ed, and it is not unmap'ed.
In a
Yasumasa,
I understand the problem, but I'm concerned of storing pointers to
invalid memory region just for coredump. Sorry!
The solution might be in another areas, e.g. print value of performance
counters to hs_err.log on crash.
-Dmitry
On 2016-05-06 14:42, Yasumasa Suenaga wrote:
> PING: Hav
PING: Have you ever reviewed it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
I've sent review request to parse core image with JSnap.
If you have unclear point about this change, please tell me.
Thanks,
Yasumasa
On 2016/04/20 21:21, Dmitry Samersoff wrote:
Yasumasa,
I
Yasumasa,
I need some more time to check what is happening with performance
counters and what side effect this fix can have.
Sorry about it.
-Dmitry
On 2016-04-20 15:14, Yasumasa Suenaga wrote:
> PING: Could you review it?
>
>>http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
>
PING: Could you review it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
This changes is based on jdk9/hs-rt.
But I confirmed this patch works fine on jdk9/hs .
Thanks,
Yasumasa
On 2016/04/13 22:22, Yasumasa Suenaga wrote:
Hi Dmitry,
Thank you for your comment.
I upload
Hi Dmitry,
Thank you for your comment.
I uploaded new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
Thanks,
Yasumasa
On 2016/04/13 21:41, Dmitry Samersoff wrote:
Yasumasa,
Yes. It's better.
Please place all _saved_* declarations together an
Yasumasa,
Yes. It's better.
Please place all _saved_* declarations together and add a comment
explaining what is the purpose of this variables.
-Dmitry
On 2016-04-13 15:20, Yasumasa Suenaga wrote:
> Hi all,
>
> Could you review and sponsor it?
>
>>http://cr.openjdk.java.net/~ysuenaga/JDK
Hi all,
Could you review and sponsor it?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.01/
If it is not accepted, I think that we can add new field to PerfMemory for
using in JSnap:
---
diff -r 4823056a5bbd src/share/vm/runtime/perfMemory.cpp
--- a/src/share/
Hi Dmitry,
Thanks for your comment.
I uploaded a new webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.01/
On 2016/04/06 0:31, Dmitry Samersoff wrote:
Yasumasa,
1. It's better to change JSnap code to produce meaningful error message
instead of NPE.
I
Yasumasa,
1. It's better to change JSnap code to produce meaningful error message
instead of NPE.
2. We should check that no other consumer of perf counters rely on the
fact it's NULL after call to destroy(). I'm not sure that this part of
the fix is not dangerous.
-Dmitry
On 2016-03-29 15:02,
PING: Could you review it?
Yasumasa
On 2016/03/22 21:24, Yasumasa Suenaga wrote:
PING: Could you review it?
Yasumasa
2016/03/14 23:39 "Yasumasa Suenaga" mailto:yasue...@gmail.com>>:
Hi all,
When I use `jhsdb jsnap` to get PerfCounter from core images, I encountered
NPE:
-
PING: Could you review it?
Yasumasa
2016/03/14 23:39 "Yasumasa Suenaga" :
> Hi all,
>
> When I use `jhsdb jsnap` to get PerfCounter from core images, I
> encountered NPE:
> -
> Exception in thread "main" java.lang.NullPointerException
> at sun.jvm.hotspot.tools.JSnap.run(JSnap
48 matches
Mail list logo
