On 5 mar 2014, at 14:17, Alan Bateman <[email protected]> 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. > > 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
