[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8036599 Use Diagnostic Commands instead of SA by default in jinfo
From: Staffan Larsen <staffan.larsen () oracle ! com>
Date: 2014-03-06 8:04:36
Message-ID: 30F55B02-CEAA-4B17-A5EC-40E229433484 () oracle ! com
[Download RAW message or body]
On 6 mar 2014, at 01:49, David Holmes <david.holmes@oracle.com> wrote:
> On 6/03/2014 2:51 AM, Staffan Larsen wrote:
> >
> > On 5 mar 2014, at 14:17, Alan Bateman <Alan.Bateman@oracle.com
> > <mailto:Alan.Bateman@oracle.com>> wrote:
> >
> > > On 04/03/2014 14:43, Staffan Larsen wrote:
> > > > The jinfo utility has three flags:
> > > > -flag: set/get value of a JVM flag
> > > > -flags: print all JVM flags
> > > > -sysprops: print all System.properties
> > > >
> > > > Currently -flags and -sysprops invokes the Serviceability Agent to
> > > > get the information. Given how intrusive the SA is this is not ideal.
> > > > I have changed the default implementation for these flags to instead
> > > > use Diagnostic Commands through the attach framework (this is also
> > > > what -flag uses). If you still want to run the SA, you can do so by
> > > > specifying -F (or by running on a core file).
> > > >
> > > > I have changed quite a bit of the (still) hairy argument parsing. The
> > > > single basic test for jinfo has also been updated so that all flags
> > > > are now exercised on all platforms (not just where SA is available).
> > > >
> > > > webrev: http://cr.openjdk.java.net/~sla/8036599/webrev.00/
> > > > bugs: https://bugs.openjdk.java.net/browse/JDK-8036599
> > > >
> > > This looks okay to me and it make sense to use the diagnostic commands
> > > (they didn't exist when jinfo was originally created).
> > >
> > > One comment on the updated usage message is that it's not emitted
> > > unconditionally so it means that there will be more options that
> > > really available when running on a build that doesn't have SA (AIX
> > > perhaps? Used to be Windows but this is no longer the case).
> >
> > Yeah, it was perhaps premature to remove it. I didn’t think we had
> > platforms without SA, but perhaps there are. I’ve added it back.
>
> The SA is only part of the full JDK not the JRE. I'm unclear whether the SA is \
> needed in the jvm running jinfo, the target jvm, or both.
SA needs nothing in the target JVM except the symbols from vmstructs.
/Staffan
>
> David
> -----
>
>
> > >
> > > A minor comment on the if-then-else-if- ... in main is that the coding
> > > style is inconsistent to the rest of the code (might be an IDE setting).
> >
> > Fixed.
> >
> > > Do you know if we have any tests that will exercise -F? Just wondering
> > > about the removal of the tests cases.
> >
> > There are other tests that exercise SA (not enough, though). I added
> > back the SA tests with a -F option.
> >
> > new webrev: http://cr.openjdk.java.net/~sla/8036599/webrev.01/
> >
> > Thanks,
> > /Staffan
[Attachment #3 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;"><br><div><div>On 6 mar 2014, at 01:49, \
David Holmes <<a href="mailto:david.holmes@oracle.com">david.holmes@oracle.com</a>> \
wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div \
style="font-size: 16px; font-style: normal; font-variant: normal; font-weight: \
normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: \
start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; \
word-spacing: 0px; -webkit-text-stroke-width: 0px;">On 6/03/2014 2:51 AM, Staffan \
Larsen wrote:<br><blockquote type="cite"><br>On 5 mar 2014, at 14:17, Alan Bateman \
<<a href="mailto:Alan.Bateman@oracle.com">Alan.Bateman@oracle.com</a><br><<a \
href="mailto:Alan.Bateman@oracle.com">mailto:Alan.Bateman@oracle.com</a>>> \
wrote:<br><br><blockquote type="cite">On 04/03/2014 14:43, Staffan Larsen \
wrote:<br><blockquote type="cite">The jinfo utility has three flags:<br>-flag: \
set/get value of a JVM flag<br>-flags: print all JVM flags<br>-sysprops: print all \
System.properties<br><br>Currently -flags and -sysprops invokes the Serviceability \
Agent to<br>get the information. Given how intrusive the SA is this is not \
ideal.<br>I have changed the default implementation for these flags to instead<br>use \
Diagnostic Commands through the attach framework (this is also<br>what -flag uses). \
If you still want to run the SA, you can do so by<br>specifying -F (or by running on \
a core file).<br><br>I have changed quite a bit of the (still) hairy argument \
parsing. The<br>single basic test for jinfo has also been updated so that all \
flags<br>are now exercised on all platforms (not just where SA is \
available).<br><br>webrev: <a \
href="http://cr.openjdk.java.net/~sla/8036599/webrev.00/">http://cr.openjdk.java.net/~sla/8036599/webrev.00/</a><br>bugs: \
<a href="https://bugs.openjdk.java.net/browse/JDK-8036599">https://bugs.openjdk.java.net/browse/JDK-8036599</a><br><br></blockquote>This \
looks okay to me and it make sense to use the diagnostic commands<br>(they didn't \
exist when jinfo was originally created).<br><br>One comment on the updated usage \
message is that it's not emitted<br>unconditionally so it means that there will be \
more options that<br>really available when running on a build that doesn't have SA \
(AIX<br>perhaps? Used to be Windows but this is no longer the \
case).<br></blockquote><br>Yeah, it was perhaps premature to remove it. I didn’t \
think we had<br>platforms without SA, but perhaps there are. I’ve added it \
back.<br></blockquote><br>The SA is only part of the full JDK not the JRE. I'm \
unclear whether the SA is needed in the jvm running jinfo, the target jvm, or \
both.<br></div></blockquote><div><br></div><div>SA needs nothing in the target JVM \
except the symbols from \
vmstructs.</div><div><br></div><div>/Staffan</div><br><blockquote type="cite"><div \
style="font-size: 16px; font-style: normal; font-variant: normal; font-weight: \
normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: \
start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; \
word-spacing: 0px; -webkit-text-stroke-width: \
0px;"><br>David<br>-----<br><br><br><blockquote type="cite"><blockquote \
type="cite"><br>A minor comment on the if-then-else-if- ... in main is that the \
coding<br>style is inconsistent to the rest of the code (might be an IDE \
setting).<br></blockquote><br>Fixed.<br><br><blockquote type="cite">Do you know if we \
have any tests that will exercise -F? Just wondering<br>about the removal of the \
tests cases.<br></blockquote><br>There are other tests that exercise SA (not enough, \
though). I added<br>back the SA tests with a -F option.<br><br>new webrev: <a \
href="http://cr.openjdk.java.net/~sla/8036599/webrev.01/">http://cr.openjdk.java.net/~ \
sla/8036599/webrev.01/</a><br><br>Thanks,<br>/Staffan</blockquote></div></blockquote></div><br></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic