PING: RFR: JDK-8151815: Could not parse core image with JSnap.
Yasumasa Suenaga
yasuenag at gmail.com
Fri May 6 11:42:32 UTC 2016
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