Re: RFR: 8151181: Add JSnap to jhsdb

[Yasumasa Suenaga]

Hi Dmitry,

I uploaded new webrev:

  hotspot (not changed): 
http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.02/hotspot/
jdk: 
http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.02/jdk/

Could you review it again?


BTW, jmap test uses "compiler detected" as expected message.
Should I fix it? (Should I file it as another test bug?)


Thanks,

Yasumasa


On 2016/03/04 2:06, Dmitry Samersoff wrote:
> Yasumasa,
> 
> 91 s/perfromance/performance/
> 
> Test:
> 
> 146 launch("compiler detected.", "jsnap");
> 
> 
> Please, choose expectedMessage carefully, I'm not sure "compiler
> detected." is an appropriate one.
> 
> -Dmitry
> 
> 
> On 2016-03-03 18:45, Yasumasa Suenaga wrote:
>> Hi Dmitry,
>>
>> Thank you for your comment.
>> I uploaded new webrev:
>>
>>hotspot: 
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.01/hotspot/
>>jdk: http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.01/jdk/
>>
>>
>> Could you review again?
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2016/03/04 0:15, Dmitry Samersoff wrote:
>>> Yasumasa,
>>>
>>> It's better (with upcoming changes for JDK-8148659 in mind) to create a
>>> separate
>>>
>>> private static boolean jsnapHelp() {
>>>  System.out.println(" \tdump perfromance counters");
>>>  return commonHelp();
>>> }
>>>
>>> and write:
>>>
>>> System.out.println(" jsnap --help\tto get more information");
>>>
>>> Please also update:
>>>
>>>  jdk/test/sun/tools/jhsdb/BasicLauncherTest.java
>>>
>>> -Dmitry
>>>
>>>
>>> On 2016-03-03 17:43, Yasumasa Suenaga wrote:
 Hi all,

 JSnap is useful SA tool to check PerfCounter.
 So I want to add it to jhsdb.

 http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.00/

 Could you review it?


 Thanks,

 Yasumasa

>>>
>>>
> 
> 

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Daniel D. Daugherty]

On 3/3/16 12:56 PM, Andreas Eriksson wrote:



On 2016-03-03 18:29, Daniel D. Daugherty wrote:

> Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

test/com/sun/jdi/RedefineAddPrivateMethod.sh
L38: System.out.println("@1 breakpoint");
L39: System.out.println("@2 breakpoint");

With these tests, this is usually done like this:

System.out.println("stop here for breakpoint 1");  // @1 
breakpoint
System.out.println("stop here for breakpoint 2");  // @2 
breakpoint


I _think_ the scaffold stuff can handle the way you did
it on all platforms, but I'm not sure. Sorry, my memory
is a bit rusty on this stuff.

Thumbs up on the original version or you can change it to
the above. Your choice.


Sorry, saw this after I had already pushed with the original version.
But other tests did similar things:
RedefineException.sh - line 83: System.out.println("a3: @1 breakpoint 
here a3");

And it ran fine through RBT, so hopefully it should be no problem.


Sounds good to me...






Dan

P.S.

It occurs to me that in the original code the main() method
is empty, i.e., just comment lines. I have a vague memory
about empty methods being treated differently in HotSpot,
but I don't remember the exact details...



What was weird is that it only failed sometimes. I'll take a look 
tomorrow to see if I can find out why, but I probably wont spend too 
much time on it.


In the original .jtr file:

Java HotSpot(TM) 64-Bit Server VM (fastdebug build 
9-internal+0-2016-03-02-004742.mgronlun.upstream, mixed mode)


Since we're in -Xmixed mode, the compiler could kick in, but
on such a short-lived test? I'm dubious...

Good hunting tomorrow!

Dan




Thanks,
Andreas




On 3/3/16 10:05 AM, Andreas Eriksson wrote:

Thanks Serguei.
I'll go ahead and push this now, since I believe this change is 
small enough.


- Andreas

On 2016-03-03 17:54, serguei.spit...@oracle.com wrote:

Hi Andreas,

Good++

Thanks,
Serguei


On 3/3/16 06:35, Dmitry Samersoff wrote:

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails 
intermittently

https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change 
the
test has not failed once after a couple of hours of testing. 
Before the

change it would fail after ~5 minutes of running it in a loop.

Thanks,
Andreas

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Andreas Eriksson]

On 2016-03-03 18:29, Daniel D. Daugherty wrote:

> Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

test/com/sun/jdi/RedefineAddPrivateMethod.sh
L38: System.out.println("@1 breakpoint");
L39: System.out.println("@2 breakpoint");

With these tests, this is usually done like this:

System.out.println("stop here for breakpoint 1");  // @1 
breakpoint
System.out.println("stop here for breakpoint 2");  // @2 
breakpoint


I _think_ the scaffold stuff can handle the way you did
it on all platforms, but I'm not sure. Sorry, my memory
is a bit rusty on this stuff.

Thumbs up on the original version or you can change it to
the above. Your choice.


Sorry, saw this after I had already pushed with the original version.
But other tests did similar things:
RedefineException.sh - line 83: System.out.println("a3: @1 breakpoint 
here a3");

And it ran fine through RBT, so hopefully it should be no problem.



Dan

P.S.

It occurs to me that in the original code the main() method
is empty, i.e., just comment lines. I have a vague memory
about empty methods being treated differently in HotSpot,
but I don't remember the exact details...



What was weird is that it only failed sometimes. I'll take a look 
tomorrow to see if I can find out why, but I probably wont spend too 
much time on it.


Thanks,
Andreas




On 3/3/16 10:05 AM, Andreas Eriksson wrote:

Thanks Serguei.
I'll go ahead and push this now, since I believe this change is small 
enough.


- Andreas

On 2016-03-03 17:54, serguei.spit...@oracle.com wrote:

Hi Andreas,

Good++

Thanks,
Serguei


On 3/3/16 06:35, Dmitry Samersoff wrote:

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change the
test has not failed once after a couple of hours of testing. 
Before the

change it would fail after ~5 minutes of running it in a loop.

Thanks,
Andreas

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Daniel D. Daugherty]

> Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

test/com/sun/jdi/RedefineAddPrivateMethod.sh
L38: System.out.println("@1 breakpoint");
L39: System.out.println("@2 breakpoint");

With these tests, this is usually done like this:

System.out.println("stop here for breakpoint 1");  // @1 breakpoint
System.out.println("stop here for breakpoint 2");  // @2 breakpoint

I _think_ the scaffold stuff can handle the way you did
it on all platforms, but I'm not sure. Sorry, my memory
is a bit rusty on this stuff.

Thumbs up on the original version or you can change it to
the above. Your choice.

Dan

P.S.

It occurs to me that in the original code the main() method
is empty, i.e., just comment lines. I have a vague memory
about empty methods being treated differently in HotSpot,
but I don't remember the exact details...



On 3/3/16 10:05 AM, Andreas Eriksson wrote:

Thanks Serguei.
I'll go ahead and push this now, since I believe this change is small 
enough.


- Andreas

On 2016-03-03 17:54, serguei.spit...@oracle.com wrote:

Hi Andreas,

Good++

Thanks,
Serguei


On 3/3/16 06:35, Dmitry Samersoff wrote:

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change the
test has not failed once after a couple of hours of testing. Before 
the

change it would fail after ~5 minutes of running it in a loop.

Thanks,
Andreas

Re: RFR: 8151181: Add JSnap to jhsdb

[Dmitry Samersoff]

Yasumasa,

91 s/perfromance/performance/

Test:

146 launch("compiler detected.", "jsnap");


Please, choose expectedMessage carefully, I'm not sure "compiler
detected." is an appropriate one.

-Dmitry


On 2016-03-03 18:45, Yasumasa Suenaga wrote:
> Hi Dmitry,
> 
> Thank you for your comment.
> I uploaded new webrev:
> 
>   hotspot: http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.01/hotspot/
>   jdk: http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.01/jdk/
> 
> 
> Could you review again?
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> On 2016/03/04 0:15, Dmitry Samersoff wrote:
>> Yasumasa,
>>
>> It's better (with upcoming changes for JDK-8148659 in mind) to create a
>> separate
>>
>> private static boolean jsnapHelp() {
>> System.out.println(" \tdump perfromance counters");
>> return commonHelp();
>> }
>>
>> and write:
>>
>> System.out.println(" jsnap --help\tto get more information");
>>
>> Please also update:
>>
>> jdk/test/sun/tools/jhsdb/BasicLauncherTest.java
>>
>> -Dmitry
>>
>>
>> On 2016-03-03 17:43, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> JSnap is useful SA tool to check PerfCounter.
>>> So I want to add it to jhsdb.
>>>
>>>http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.00/
>>>
>>> Could you review it?
>>>
>>>
>>> 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.

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Andreas Eriksson]

Thanks Serguei.
I'll go ahead and push this now, since I believe this change is small 
enough.


- Andreas

On 2016-03-03 17:54, serguei.spit...@oracle.com wrote:

Hi Andreas,

Good++

Thanks,
Serguei


On 3/3/16 06:35, Dmitry Samersoff wrote:

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change the
test has not failed once after a couple of hours of testing. Before the
change it would fail after ~5 minutes of running it in a loop.

Thanks,
Andreas

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[serguei.spit...@oracle.com]

Hi Andreas,

Good++

Thanks,
Serguei


On 3/3/16 06:35, Dmitry Samersoff wrote:

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change the
test has not failed once after a couple of hours of testing. Before the
change it would fail after ~5 minutes of running it in a loop.

Thanks,
Andreas

RE: RFR[9u-dev]: 8146683: check_addr0 should be more efficient

[Cheleswer Sahu]

Hi,

Please review the new code changes for this bug. I have removed  " fstat()"  
call which seems not safe to read the "/proc/map/self" file and have made some 
other changes too. Please find the latest code in below link
Webrev link: http://cr.openjdk.java.net/~csahu/8146683/webrev.01/


Regards,
Cheleswer

-Original Message-
From: Dean Long 
Sent: Wednesday, February 24, 2016 7:44 AM
To: Daniel Daugherty; Thomas Stüfe; Cheleswer Sahu
Cc: serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR[9u-dev]: 8146683: check_addr0 should be more efficient

On 2/23/2016 10:07 AM, Daniel D. Daugherty wrote:
> On 2/23/16 5:56 AM, Thomas Stüfe wrote:
>> Hi Cheleswer,
>>
>>
>> On Tue, Feb 23, 2016 at 9:38 AM, Cheleswer Sahu 
>> 
>>   wrote:
>>
>>> Hi,
>>>
>>>
>>>
>>> Please review the code changes for
>>> https://bugs.openjdk.java.net/browse/JDK-8146683 .
>>>
>>>
>>>
>>> Webrev link: http://cr.openjdk.java.net/~csahu/8146683/
>>>
>>> JBS Link: https://bugs.openjdk.java.net/browse/JDK-8146683
>>>
>>>
>>>
>>> Bug Brief:
>>>
>>> While investigating  bug
>>> https://bugs.openjdk.java.net/browse/JDK-8144483
>>> it has been observed that "check_addr0() " function  is not written 
>>> efficiently.
>>>
>>> This function is trying to read each "prmap_t" entry in 
>>> "/proc/self/map"
>>> file one by one which is time taking and can cause in long pauses.
>>>
>>>
>>>
>>> Solution proposed:
>>>
>>> Instead of reading each "prmap_t" entry in "/proc/self/map" file one 
>>> by one, read the entries in chunks. There can be many entries in "map"
>>> file,
>>> so I have decided to read these
>>>
>>> in chunks of 100  "prmap_t" entries. Reading entries in chunks saves 
>>> a lot of read calls and results in smaller pause times.
>>>
>>>
>>>
>>>
>>>
>>> Regards,
>>>
>>> Cheleswer
>>>
>>
>> We saw cases, especially on Solaris, where the content of a file in the
>> proc file system changed under us while we were reading it. So this 
>> should
>> be kept in mind. The original code seems also broken in that regard,
>> because it assumes the ::read() call to read the full size of a prmap_t
>> entry, but it may theoretically read an incomplete entry.
>>
>> As for your coding, you first estimate the size of the mapping file and
>> then use this to determine how many entries to read; but when 
>> reading, this
>> information may already be stale. I think it would be more robust and 
>> also
>> simpler to just try to read as many entries as possible - in chunks, to
>> make reading faster - until you get EOF.
>>
>> Kind Regards, Thomas
>
> I'm really sure that we've been down this road before. In particular,
> Dmitry Samersoff has worked on this issue (or one very much like it)
> before, but he is on vacation until the end of the month.
>

There was a similar issue with Linux /proc/self/maps, and whether it was 
safe to read the file with buffered stdio or not.  See 8009062 and 7017193.

dl

> Dan
>
>
>>
>> On Tue, Feb 23, 2016 at 9:38 AM, Cheleswer Sahu 
>> 
>> wrote:
>>
>>> Hi,
>>>
>>>
>>>
>>> Please review the code changes for
>>> https://bugs.openjdk.java.net/browse/JDK-8146683 .
>>>
>>>
>>>
>>> Webrev link: http://cr.openjdk.java.net/~csahu/8146683/
>>>
>>> JBS Link: https://bugs.openjdk.java.net/browse/JDK-8146683
>>>
>>>
>>>
>>> Bug Brief:
>>>
>>> While investigating  bug 
>>> https://bugs.openjdk.java.net/browse/JDK-8144483
>>> it has been observed that "check_addr0() " function  is not written
>>> efficiently.
>>>
>>> This function is trying to read each "prmap_t" entry in 
>>> "/proc/self/map"
>>> file one by one which is time taking and can cause in long pauses.
>>>
>>>
>>>
>>> Solution proposed:
>>>
>>> Instead of reading each "prmap_t" entry in "/proc/self/map" file one by
>>> one, read the entries in chunks. There can be many entries in "map" 
>>> file,
>>> so I have decided to read these
>>>
>>> in chunks of 100  "prmap_t" entries. Reading entries in chunks saves 
>>> a lot
>>> of read calls and results in smaller pause times.
>>>
>>>
>>>
>>>
>>>
>>> Regards,
>>>
>>> Cheleswer
>>>
>

Re: RFR: 8151181: Add JSnap to jhsdb

[Yasumasa Suenaga]

Hi Dmitry,

Thank you for your comment.
I uploaded new webrev:

  hotspot: http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.01/hotspot/
  jdk: http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.01/jdk/


Could you review again?


Thanks,

Yasumasa


On 2016/03/04 0:15, Dmitry Samersoff wrote:
> Yasumasa,
> 
> It's better (with upcoming changes for JDK-8148659 in mind) to create a
> separate
> 
> private static boolean jsnapHelp() {
> System.out.println(" \tdump perfromance counters");
> return commonHelp();
> }
> 
> and write:
> 
> System.out.println(" jsnap --help\tto get more information");
> 
> Please also update:
> 
> jdk/test/sun/tools/jhsdb/BasicLauncherTest.java
> 
> -Dmitry
> 
> 
> On 2016-03-03 17:43, Yasumasa Suenaga wrote:
>> Hi all,
>>
>> JSnap is useful SA tool to check PerfCounter.
>> So I want to add it to jhsdb.
>>
>>http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.00/
>>
>> Could you review it?
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
> 
> 

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Andreas Eriksson]

Thanks.

- Andreas

On 2016-03-03 15:35, Dmitry Samersoff wrote:

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change the
test has not failed once after a couple of hours of testing. Before the
change it would fail after ~5 minutes of running it in a loop.

Thanks,
Andreas

Re: RFR: 8151181: Add JSnap to jhsdb

[Dmitry Samersoff]

Yasumasa,

It's better (with upcoming changes for JDK-8148659 in mind) to create a
separate

private static boolean jsnapHelp() {
   System.out.println(" \tdump perfromance counters");
   return commonHelp();
}

and write:

System.out.println(" jsnap --help\tto get more information");

Please also update:

   jdk/test/sun/tools/jhsdb/BasicLauncherTest.java

-Dmitry


On 2016-03-03 17:43, Yasumasa Suenaga wrote:
> Hi all,
> 
> JSnap is useful SA tool to check PerfCounter. 
> So I want to add it to jhsdb.
> 
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.00/
> 
> Could you review it?
> 
> 
> 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.

Re: JDK-8148659: Add all option to JSnap

[Yasumasa Suenaga]

Hi Dmitry,

> Overall direction is to support jhsdb as the only entry point for SA and
> all coredump related staff.

I'm working on another issue (JDK-8151181) for it.
  
http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-March/019007.html

After that, I will work for this issue (JDK-8148659) and adding all option
to JSnap.
  
http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-March/018962.html


Thanks,

Yasumasa


On 2016/03/03 22:55, Dmitry Samersoff wrote:
> Yasumasa,
> 
> Overall direction is to support jhsdb as the only entry point for SA and
> all coredump related staff.
> 
> Could you move JSnap "into jhsdb".
> 
> And this change requires CCC approval (I'll file it for you)
> 
> -Dmitry
> 
> On 2016-01-30 18:48, Yasumasa Suenaga wrote:
>> I want to add -a (all) option to JSnap because JSnap is very useful to 
>> analyze core image.
>>
>> Currently, JSnap can show PerfDataEntry which namespace is java.* or 
>> com.sun.* .
>> We can use jcmd with PerfCounter.print when we want to attach live process, 
>> however,
>> as JDK-6224040 shows, we have to use JSnap.
>>
>> In terms of core image analysis, -a option is very useful for 
>> troubleshooting.
>>
>> I uploaded a webrev for this enhancement.
>> Could you review it?
>>
>>http://cr.openjdk.java.net/~ysuenaga/JDK-8148659/webrev.00/
>>
>> I'm jdk9 committer, however I cannot access JPRT.
>> So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
> 
> 

RFR: 8151181: Add JSnap to jhsdb

[Yasumasa Suenaga]

Hi all,

JSnap is useful SA tool to check PerfCounter. 
So I want to add it to jhsdb.

  http://cr.openjdk.java.net/~ysuenaga/JDK-8151181/webrev.00/

Could you review it?


Thanks,

Yasumasa

Re: RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Dmitry Samersoff]

Andreas,

Looks good for me.

-Dmitry

On 2016-03-03 17:05, Andreas Eriksson wrote:
> Hi,
> 
> Can I please have a review of this fix for
> 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
> https://bugs.openjdk.java.net/browse/JDK-8151064
> 
> Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/
> 
> Still not sure why it only fails sometimes, but after this change the
> test has not failed once after a couple of hours of testing. Before the
> change it would fail after ~5 minutes of running it in a loop.
> 
> Thanks,
> Andreas


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.

RFR (XS): 8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently

[Andreas Eriksson]

Hi,

Can I please have a review of this fix for
8151064: com/sun/jdi/RedefineAddPrivateMethod.sh fails intermittently
https://bugs.openjdk.java.net/browse/JDK-8151064

Webrev: http://cr.openjdk.java.net/~aeriksso/8151064/webrev/

Still not sure why it only fails sometimes, but after this change the 
test has not failed once after a couple of hours of testing. Before the 
change it would fail after ~5 minutes of running it in a loop.


Thanks,
Andreas

Re: JDK-8148659: Add all option to JSnap

[Dmitry Samersoff]

Yasumasa,

Overall direction is to support jhsdb as the only entry point for SA and
all coredump related staff.

Could you move JSnap "into jhsdb".

And this change requires CCC approval (I'll file it for you)

-Dmitry

On 2016-01-30 18:48, Yasumasa Suenaga wrote:
> I want to add -a (all) option to JSnap because JSnap is very useful to 
> analyze core image.
> 
> Currently, JSnap can show PerfDataEntry which namespace is java.* or 
> com.sun.* .
> We can use jcmd with PerfCounter.print when we want to attach live process, 
> however,
> as JDK-6224040 shows, we have to use JSnap.
> 
> In terms of core image analysis, -a option is very useful for troubleshooting.
> 
> I uploaded a webrev for this enhancement.
> Could you review it?
> 
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8148659/webrev.00/
> 
> I'm jdk9 committer, however 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.

Re: G1 STW phases and FGC column in jstat

[Yasumasa Suenaga]

Hi all,

I've uploaded PoC of PerfCounter for STW phase in concurrent GC:

  hotspot: http://cr.openjdk.java.net/~ysuenaga/perfcounter-for-cgc/hotspot/
  jdk: http://cr.openjdk.java.net/~ysuenaga/perfcounter-for-cgc/jdk/

These patches makes new jstat column CGC and CGCT as below:
-
$ jstat -gc 53494 3000
 S0CS1CS0US1U  EC   EUOC OU   MC MU 
   CCSC   CCSU   YGC YGCTFGCFGCTCGCCGCT GCT
 0.00.00.00.04096.00.0 28672.0  0.0 4480.0 
948.9  384.0   77.6   00.000   0  0.000   0  0.0000.000
 0.0   1024.0  0.0   1024.0  3072.00.0 28672.0  0.0 4864.0 
2969.0 512.0  276.0   10.063   0  0.000   2  0.0160.080
-

They are shows STW phase in concurrent GC.

  CMS: Initial Mark and Remark
   G1: Remark and Cleanup

GCT is sum of YGCT, FGCT, and CGCT.


Can be this change accepted?
If so, I will file it to JBS and will send RFR.

I cannot access JPRT.
So I need a sponsor.


Thanks,

Yasumasa


On 2016/03/02 23:19, Yasumasa Suenaga wrote:

It is not difficult to create webrev for this proposal.
However, this proposal affects to jstat output, and
I cannot access JPRT.
So I want to find sponsor for it at first.

Could somebody be a sponsor?


Yasumasa


On 2016/03/02 22:29, k...@kodewerk.com wrote:

Hi Yasumasa,

It all sounds reasonable and of course the impact on downstream tooling vs the 
value of the change must be evaluated. In this case I think adding a CGC 
counter makes sense especially in prep for G1 becoming default in 9.

Regards,
Kirk


On Mar 2, 2016, at 6:58 AM, Yasumasa Suenaga  wrote:

Hi Kirk,

Agree to you.
However, I use occasional FGC counter at CMS as below:

1. Check major collection occurrence
  Some production systems have large memory as Java heap,
  and they are not set GC log.
  If their CPU usage becomes high, I want to check GC execution.
  (Of course, we have to check any other points :-) )

2. Core image analysis
  If JVM is crashed, I want to check PerfCounter to know situation.
  (In the past, I sometimes encountered crash at GC worker thread.)

I guess that I will want to check them at G1.

Thus, at least, I want to add PerfCounter for CGC (and add JVMTI event hook).
However, this proposal will affect to jstat spec.
So I want to discuss about it before filing to JBS.


Thanks,

Yasumasa


On 2016/03/02 19:02, k...@kodewerk.com wrote:

Hi Yasumasa,

Good question. I’ve never considered CMS to be a Full GC. This implies that 
there should be separate performance counters for CMS pause phases as it is 
possible to have FGC. Of course a FGC during CMS maybe user triggered, 
triggered outside a CMS cycle, interrupts a CMS cycle, or interrupts a CMS 
phase. I’m not sure how much of a distinction one needs to make here as that 
could be a quick broader discussion. Certainly the purpose isn’t to recreate 
the GC logs in these performance counters. But at the very least not having a 
distinction between full and a STW-CMS phase is kind of misleading in my 
opinion.

Regards,
Kirk


On Mar 1, 2016, at 5:03 PM, Yasumasa Suenaga  wrote:

Hi Kirk,


It is also incorrect to count initial mark and remark in CMS as a FGC.


Though, how can we check execution of major collection without GC log?
Should we add new PerfCounter for CGC (and add CGC column to jstat output)?


Yasumasa


On 2016/03/02 6:35, k...@kodewerk.com wrote:

Hi,

I think it is incorrect to count remark and cleanup as FGC. They are not full 
collections. It is also incorrect to count initial mark and remark in CMS as a 
FGC. It is unfortunate that this is counted this way.

Regards,
Kirk


On Mar 1, 2016, at 8:56 AM, Yasumasa Suenaga  wrote:

Hi all,

I wonder that STW phases (Remark and Cleanup) at G1 are not counted in jstat 
FGC column.
For example, Initial Mark and Remark at CMS are counted as FGC.

For consistency, I think that G1 STW phases should be counted as FGC.
What do you think about it?

If it is accepted, I will file it to JBS and will upload webrev.


suggested fix:
--
diff -r 8a103ba9a7b2 src/share/vm/gc/g1/g1MonitoringSupport.cpp
--- a/src/share/vm/gc/g1/g1MonitoringSupport.cppMon Feb 29 22:54:24 2016 
+0900
+++ b/src/share/vm/gc/g1/g1MonitoringSupport.cppTue Mar 01 23:43:30 2016 
+0900
@@ -103,7 +103,7 @@
   //   name "collector.1".  In a generational collector this would be the
   // old generation collection.
   _full_collection_counters =
-new CollectorCounters("G1 stop-the-world full collections", 1);
+new CollectorCounters("G1 stop-the-world phase", 1);

   // timer sampling for all counters supporting sampling only update the
   // used value.  See the take_sample() method.  G1 requires both used and
diff -r 8a103ba9a7b2 src/share/vm/gc/g1/vm_operations_g1.cpp
--- a/src/share/vm/gc/g1/vm_operations_g1.cppMon Feb 

RE: RFR(XXS): 8151100: Test java/lang/instrument/NativeMethodPrefixAgent.java can't attempt to do CheckIntrinsics

[Markus Gronlund]

Thanks Dmitry  for noticing this - I am running slowdebug so I didn't see that.

 

Thanks again.

Markus

 

 

From: Dmitry Dmitriev 
Sent: den 3 mars 2016 12:23
To: Markus Gronlund; serviceability-dev@openjdk.java.net
Subject: Re: RFR(XXS): 8151100: Test 
java/lang/instrument/NativeMethodPrefixAgent.java can't attempt to do 
CheckIntrinsics

 

Hello Markus,

CheckIntrinsics is a diagnostic flag. I think you need to add 
-XX:+UnlockDiagnosticVMOptions before it because otherwise test will fail on 
product build.

Thanks,
Dmitry 

On 03.03.2016 3:15, Markus Gronlund wrote:

Greetings,

 

Could a please ask for reviews for the following simple fix to resolve a test 
issue associated with test/java/lang/instrument/NativeMethodPrefixAgent.java

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8151100 

 

Webrev/diff:

 

diff --git a/test/java/lang/instrument/NativeMethodPrefixAgent.java 
b/test/java/lang/instrument/NativeMethodPrefixAgent.java

--- a/test/java/lang/instrument/NativeMethodPrefixAgent.java

+++ b/test/java/lang/instrument/NativeMethodPrefixAgent.java

@@ -31,7 +31,7 @@

  *  java.management

  *  java.instrument

  * @run shell/timeout=240 MakeJAR2.sh NativeMethodPrefixAgent 
NativeMethodPrefixApp 'Can-Retransform-Classes: true' 
'Can-Set-Native-Method-Prefix: true'

- * @run main/othervm -javaagent:NativeMethodPrefixAgent.jar 
NativeMethodPrefixApp

+ * @run main/othervm -XX:-CheckIntrinsics 
-javaagent:NativeMethodPrefixAgent.jar NativeMethodPrefixApp

  */

 import java.lang.instrument.*;

 

Description:

 

The java/lang/instrument/NativeMethodPrefixAgent.java modifies (wraps) the 
names of native methods.

If a class is loaded that contain the @HotSpotIntrinsicCandidate annotation on 
a native method, one step of the class file parser is an attempt to validate 
the method name against a registered intrinsic in the VM.

 

By default, the flag CheckIntrinsics is true, which will yield:

 

"Method 
[.wrapped_tr0_wrapped_tr1_wrapped_tr2_()] 
is annotated with @HotSpotIntrinsicCandidate, but no compiler intrinsic is 
defined for the method. Exiting."

This "wrapped" prepending to the method name invalidates the intrinsic check. 

 

In order for the test to pass even when loading a class with a native method 
containing the @HotSpotIntrinsicCandidate annotation, the test should 
explicitly disable the CheckIntrinsics flag: 

 

-XX:-CheckIntrinsics

 

Thanks in advance

Markus

 

Re: RFR(XXS): 8151100: Test java/lang/instrument/NativeMethodPrefixAgent.java can't attempt to do CheckIntrinsics

[Dmitry Dmitriev]

Hello Markus,

CheckIntrinsics is a diagnostic flag. I think you need to add 
-XX:+UnlockDiagnosticVMOptions before it because otherwise test will 
fail on product build.


Thanks,
Dmitry

On 03.03.2016 3:15, Markus Gronlund wrote:


Greetings,

Could a please ask for reviews for the following simple fix to resolve 
a test issue associated with 
test/java/lang/instrument/NativeMethodPrefixAgent.java


Bug: https://bugs.openjdk.java.net/browse/JDK-8151100

Webrev/diff:

diff --git a/test/java/lang/instrument/NativeMethodPrefixAgent.java 
b/test/java/lang/instrument/NativeMethodPrefixAgent.java


--- a/test/java/lang/instrument/NativeMethodPrefixAgent.java

+++ b/test/java/lang/instrument/NativeMethodPrefixAgent.java

@@ -31,7 +31,7 @@

  * java.management

  * java.instrument

  * @run shell/timeout=240 MakeJAR2.sh NativeMethodPrefixAgent 
NativeMethodPrefixApp 'Can-Retransform-Classes: true' 
'Can-Set-Native-Method-Prefix: true'


- * @run main/othervm -javaagent:NativeMethodPrefixAgent.jar 
NativeMethodPrefixApp


+ * @run main/othervm -XX:-CheckIntrinsics 
-javaagent:NativeMethodPrefixAgent.jar NativeMethodPrefixApp


*/

 import java.lang.instrument.*;

Description:

The java/lang/instrument/NativeMethodPrefixAgent.java modifies (wraps) 
the names of native methods.


If a class is loaded that contain the @HotSpotIntrinsicCandidate 
annotation on a native method, one step of the class file parser is an 
attempt to validate the method name against a registered intrinsic in 
the VM.


By default, the flag CheckIntrinsics is true, which will yield:

“Method 
[.wrapped_tr0_wrapped_tr1_wrapped_tr2_()] 
is annotated with @HotSpotIntrinsicCandidate, but no compiler 
intrinsic is defined for the method. Exiting.”


This "wrapped" prepending to the method name invalidates the intrinsic 
check.


In order for the test to pass even when loading a class with a native 
method containing the @HotSpotIntrinsicCandidate annotation, the test 
should explicitly disable the CheckIntrinsics flag:


-XX:-CheckIntrinsics

Thanks in advance

Markus

Re: RFR(XXS): 8151100: Test java/lang/instrument/NativeMethodPrefixAgent.java can't attempt to do CheckIntrinsics

[serguei.spit...@oracle.com]

Hi Markus,

The fix looks good.

Thanks,
Serguei


On 3/2/16 16:15, Markus Gronlund wrote:


Greetings,

Could a please ask for reviews for the following simple fix to resolve 
a test issue associated with 
test/java/lang/instrument/NativeMethodPrefixAgent.java


Bug: https://bugs.openjdk.java.net/browse/JDK-8151100

Webrev/diff:

diff --git a/test/java/lang/instrument/NativeMethodPrefixAgent.java 
b/test/java/lang/instrument/NativeMethodPrefixAgent.java


--- a/test/java/lang/instrument/NativeMethodPrefixAgent.java

+++ b/test/java/lang/instrument/NativeMethodPrefixAgent.java

@@ -31,7 +31,7 @@

  * java.management

  * java.instrument

  * @run shell/timeout=240 MakeJAR2.sh NativeMethodPrefixAgent 
NativeMethodPrefixApp 'Can-Retransform-Classes: true' 
'Can-Set-Native-Method-Prefix: true'


- * @run main/othervm -javaagent:NativeMethodPrefixAgent.jar 
NativeMethodPrefixApp


+ * @run main/othervm -XX:-CheckIntrinsics 
-javaagent:NativeMethodPrefixAgent.jar NativeMethodPrefixApp


*/

 import java.lang.instrument.*;

Description:

The java/lang/instrument/NativeMethodPrefixAgent.java modifies (wraps) 
the names of native methods.


If a class is loaded that contain the @HotSpotIntrinsicCandidate 
annotation on a native method, one step of the class file parser is an 
attempt to validate the method name against a registered intrinsic in 
the VM.


By default, the flag CheckIntrinsics is true, which will yield:

“Method 
[.wrapped_tr0_wrapped_tr1_wrapped_tr2_()] 
is annotated with @HotSpotIntrinsicCandidate, but no compiler 
intrinsic is defined for the method. Exiting.”


This "wrapped" prepending to the method name invalidates the intrinsic 
check.


In order for the test to pass even when loading a class with a native 
method containing the @HotSpotIntrinsicCandidate annotation, the test 
should explicitly disable the CheckIntrinsics flag:


-XX:-CheckIntrinsics

Thanks in advance

Markus

Re: PING Re: RFR(s): 8150026: Add the ability to log with variable log level

[Robbin Ehn]

Thanks Bengt!

/Robbin

On 03/03/2016 09:35 AM, Bengt Rutisson wrote:


Hi Robbin,

On 2016-03-03 07:41, Robbin Ehn wrote:

Hi,

Could someone else also please review this?

On 02/29/2016 11:22 AM, Marcus Larsson wrote:

Hi Robbin,

On 02/18/2016 08:37 PM, Robbin Ehn wrote:

Hi, please review.

This adds a write method to the UL log which takes the level as
parameter.

JBS: https://bugs.openjdk.java.net/browse/JDK-8150026
Webrev: http://cr.openjdk.java.net/~mlarsson/rehn/8150026/


Looks good, thanks for fixing this.


Looks good to me too.

Bengt



Marcus


Thanks Marcus!

/R





Tested ok with rbt hotspot_all.

Thanks!

/Robbin

Re: PING Re: RFR(s): 8150026: Add the ability to log with variable log level

[Bengt Rutisson]

Hi Robbin,

On 2016-03-03 07:41, Robbin Ehn wrote:

Hi,

Could someone else also please review this?

On 02/29/2016 11:22 AM, Marcus Larsson wrote:

Hi Robbin,

On 02/18/2016 08:37 PM, Robbin Ehn wrote:

Hi, please review.

This adds a write method to the UL log which takes the level as
parameter.

JBS: https://bugs.openjdk.java.net/browse/JDK-8150026
Webrev: http://cr.openjdk.java.net/~mlarsson/rehn/8150026/


Looks good, thanks for fixing this.


Looks good to me too.

Bengt



Marcus


Thanks Marcus!

/R





Tested ok with rbt hotspot_all.

Thanks!

/Robbin