request for codereview, bug 7149528: easy usage of serviceability agent

[Staffan Larsen]

Yumin,

Thanks for doing this!

I still wonder if we should support -startsvr in the new launcher, given that we already have a jsadebugd launcher that does exactly that. I don't like having two ways of doing the same thing that we have to support. Either we have jsadebugd or we have "jsa -startsvr", but I think it will be a lot of work to remove jsadebugd so it's simpler to leave it and remove the -startsvr flag.

I think the usage instructions could be made easier to read by splitting them on multiple lines for the "live process" case and the "core" case. See the output from jsadebugd for an example. It would also be good if these two tools used the same terminology.

A couple of nits for SAMain:
* the case statements do not need parentheses around the value: "case (0):" should be "case 0:"
* line 35: missing space before {
* line 37: missing space after the if
* line 37: always use {} and newlines after if
* line 42 & 43: I prefer having the method implementation on a new line after the declaration

Thanks,
/Staffan



On 11 maj 2012, at 23:48, Yumin Qi wrote:

> Hi, all
> 
>  Can I have your codereview for bug 7149528: easy usage of serviceability agent.
>  Problems 1) for launching Serviceability Agent(SA) is that it needs setting several environmental variables for class path, library path options. It is not easy for user to become familiar with those settings. 2) SA launched from java and attached to java process (or the binary which loaded JVM). On MacOS, attaching to process needs permission. If we grant access for SA, we have to grant all java processes have the same access permission so brings security concern on the platform since java is too generic for doing so. With a separate SA launcher, giving it permission attaching to java process solves the concern.
> 
> the change for both hotspot and jdk:
> http://cr.openjdk.java.net/~minqi/7149528
> 
> Thanks
> Yumin