PING: RFR: JDK-8151815: Could not parse core image with JSnap.

[Yasumasa Suenaga]

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
>>>>>>>
>>>>>
>>>>>
>>
>>