PING: RFR: JDK-8151815: Could not parse core image with JSnap.
Yasumasa Suenaga
yasuenag at gmail.com
Fri May 6 13:39:26 UTC 2016
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 cannot get hs_err log.
I posted a bug report for this issue as JDK-7109520, but it was rejected.
In all crash case, we might collect core image.
If I collect it, I want to check performance counter in it.
Thus I want to merge this patch to JDK 9.
Is it difficult?
Thanks,
Yasumasa
On 2016/05/06 21:42, Yasumasa Suenaga wrote:
> 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 addition, webrev.02 adds several pointers to store addresses which are
> related to PerfMemory.
> It is not disruptive against current memory management which is related to
> PerfMemory.
>
> Coredump will include all virtual memory and thread contexts.
> If pointers ( _saved_* in webrev.02) are invalid, it is not affect JVM behavior,
> but JSnap will fail.
> (This pointer is referred from JSnap only.)
>
>
>> 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.
>
>
> Thanks,
>
> Yasumasa
>
>
>> -Dmitry
>>
>>
>> On 2016-05-06 14:42, Yasumasa Suenaga wrote:
>>> 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 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/
>>>>>
>>>>> 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 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 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-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/vm/runtime/perfMemory.cpp Tue Apr 12 09:08:48
>>>>>>>> 2016 +0000
>>>>>>>> +++ b/src/share/vm/runtime/perfMemory.cpp Wed Apr 13 14:18:15
>>>>>>>> 2016 +0900
>>>>>>>> @@ -45,11 +45,16 @@
>>>>>>>> UINT_CHARS + 1;
>>>>>>>>
>>>>>>>> char* PerfMemory::_start = NULL;
>>>>>>>> +char* PerfMemory::_saved_start = NULL;
>>>>>>>> char* PerfMemory::_end = NULL;
>>>>>>>> +char* PerfMemory::_saved_end = NULL;
>>>>>>>> char* PerfMemory::_top = NULL;
>>>>>>>> +char* PerfMemory::_saved_top = NULL;
>>>>>>>> size_t PerfMemory::_capacity = 0;
>>>>>>>> +size_t PerfMemory::_saved_capacity = 0;
>>>>>>>> jint PerfMemory::_initialized = false;
>>>>>>>> PerfDataPrologue* PerfMemory::_prologue = NULL;
>>>>>>>> +PerfDataPrologue* PerfMemory::_saved_prologue = NULL;
>>>>>>>>
>>>>>>>> void perfMemory_init() {
>>>>>>>>
>>>>>>>> @@ -103,6 +108,8 @@
>>>>>>>>
>>>>>>>> // allocate PerfData memory region
>>>>>>>> create_memory_region(capacity);
>>>>>>>> + _saved_start = _start;
>>>>>>>> + _saved_capacity = _capacity;
>>>>>>>>
>>>>>>>> if (_start == NULL) {
>>>>>>>>
>>>>>>>> @@ -132,8 +139,11 @@
>>>>>>>> }
>>>>>>>>
>>>>>>>> _prologue = (PerfDataPrologue *)_start;
>>>>>>>> + _saved_prologue = _prologue;
>>>>>>>> _end = _start + _capacity;
>>>>>>>> + _saved_end = _end;
>>>>>>>> _top = _start + sizeof(PerfDataPrologue);
>>>>>>>> + _saved_top = _top;
>>>>>>>> }
>>>>>>>>
>>>>>>>> assert(_prologue != NULL, "prologue pointer must be
>>>>>>>> initialized");
>>>>>>>> -----------------------
>>>>>>>>
>>>>>>>> If it is better, I will upload a webrev.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2016/04/06 12:42, Yasumasa Suenaga wrote:
>>>>>>>>> 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 added null check and set message to PerfMemory.java .
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>> I added new flag (_destroyed) in PerfMemory class.
>>>>>>>>> This flag set true at PerfMemory::destroy().
>>>>>>>>>
>>>>>>>>> _start, _end, and more field which I removed from destroy() are
>>>>>>>>> private member of
>>>>>>>>> PerfMemory. I implemented that the getter functions of them check
>>>>>>>>> _destroyed flag,
>>>>>>>>> and returns same value before this change.
>>>>>>>>>
>>>>>>>>> So I think this change does not affect other place.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -Dmitry
>>>>>>>>>>
>>>>>>>>>> On 2016-03-29 15:02, Yasumasa Suenaga wrote:
>>>>>>>>>>> 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" <yasuenag at gmail.com
>>>>>>>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>>>>>>>>
>>>>>>>>>>>> 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.java:45)
>>>>>>>>>>>> at
>>>>>>>>>>>> sun.jvm.hotspot.tools.Tool.startInternal(Tool.java:260)
>>>>>>>>>>>> at sun.jvm.hotspot.tools.Tool.start(Tool.java:223)
>>>>>>>>>>>> at
>>>>>>>>>>>> sun.jvm.hotspot.tools.Tool.execute(Tool.java:118)
>>>>>>>>>>>> at sun.jvm.hotspot.tools.JSnap.main(JSnap.java:67)
>>>>>>>>>>>> at
>>>>>>>>>>>> sun.jvm.hotspot.SALauncher.runJSNAP(SALauncher.java:352)
>>>>>>>>>>>> at
>>>>>>>>>>>> sun.jvm.hotspot.SALauncher.main(SALauncher.java:404)
>>>>>>>>>>>> -------------
>>>>>>>>>>>>
>>>>>>>>>>>> PerfMemory::destroy() clears all members which are used in
>>>>>>>>>>>> JSnap.
>>>>>>>>>>>> Thus NPE is occurred.
>>>>>>>>>>>>
>>>>>>>>>>>> I uploaded webrev for this issue.
>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> I cannot access JPRT.
>>>>>>>>>>>> So I need a Sponsor.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>>>
>>
>>
More information about the serviceability-dev
mailing list