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" <yasue...@gmail.com
<mailto:yasue...@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





Reply via email to