[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: PING: RFR: JDK-8151815: Could not parse core image with JSnap.
From: Dmitry Samersoff <dmitry.samersoff () oracle ! com>
Date: 2016-04-20 12:21:55
Message-ID: 57177463.8060003 () oracle ! com
[Download RAW message or body]
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@gmail.com
>>>>>>>> <mailto:yasuenag@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
>>>>>>>>
>>>>>>
>>>>>>
>>>
>>>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic