jvm-profiling-tools / perf-map-agent

A java agent to generate method mappings to use with the linux `perf` tool
GNU General Public License v2.0
1.65k stars 260 forks source link

Resolve `perf-map-agent` directory better #15

Closed nobeh closed 9 years ago

nobeh commented 9 years ago

Thanks for your small but very useful tool. I've been working with the latest commits from master on the tool and I hit some issues mainly related to how the paths are solved and the main JAR file is executed. Here is a suggest to improve a bit for simpler usage:

diff --git a/bin/create-java-perf-map.sh b/bin/create-java-perf-map.sh
index 604fa44..b83f342 100755
--- a/bin/create-java-perf-map.sh
+++ b/bin/create-java-perf-map.sh
@@ -6,8 +6,7 @@ CUR_DIR=`pwd`
 PID=$1
 OPTIONS=$2
 ATTACH_JAR=attach-main.jar
-PERF_MAP_DIR=$(dirname $(readlink -f $0))/..
-ATTACH_JAR_PATH=$PERF_MAP_DIR/out/$ATTACH_JAR
+PERF_MAP_DIR=$(realpath $(dirname $(readlink -f $0))/..)
 PERF_MAP_FILE=/tmp/perf-$PID.map

 if [ -z $JAVA_HOME ]; then
@@ -18,5 +17,5 @@ fi
 [ -d $JAVA_HOME ] || (echo "JAVA_HOME directory at '$JAVA_HOME' does not exist." && false)

 sudo rm $PERF_MAP_FILE -f
-(cd $PERF_MAP_DIR && java -cp $ATTACH_JAR_PATH:$JAVA_HOME/lib/tools.jar net.virtualvoid.perf.AttachOnce $PID $OPTIONS)
+(cd $PERF_MAP_DIR/out && java -cp $ATTACH_JAR:$JAVA_HOME/lib/tools.jar net.virtualvoid.perf.AttachOnce $PID $OPTIONS)
 sudo chown root:root $PERF_MAP_FILE
diff --git a/bin/perf-java-flames b/bin/perf-java-flames
index eb7c235..4a3d4ee 100755
--- a/bin/perf-java-flames
+++ b/bin/perf-java-flames
@@ -26,7 +26,7 @@ if [ -z $PERF_DATA_FILE ]; then
 fi

 if [ -z $PERF_FLAME_OUTPUT ]; then
-  PERF_FLAME_OUTPUT=flamegraph-$PID.svg
+  PERF_FLAME_OUTPUT=$PERF_JAVA_TMP/flamegraph-$PID.svg
 fi

 $PERF_MAP_DIR/bin/perf-java-record-stack $*

Summary:

jrudolph commented 9 years ago

Thanks for the suggestions. What kind of issues did you have?

The reason that PERF_FLAME_OUTPUT defaults to the current directory is that the output file is a "real product" of the call so I wouldn't want it to default to a possibly transient directory.

nobeh commented 9 years ago

I agree with the argument. Here is how I used the tool:

$ # clone the repo
$ # install
$ # did NOT create the symlinks
$ launch my app on a different term
$ cd /path/to/perf-map-agent
$ ./bin/perf-java-flames $PID

With the suggestions to resolve the real path of perf-map-agent, in the end, I got the result in root directory of perf-map-agent. I agree this is not critical and due to how I used the tool. However, I would also not want to pollute the git repo for perf-map-agent when using it.

Without the suggestions to resolve the real path of `perf-map-agent, I actually got an exception about failing to load the agent which makes sense because of how I used the tool.

jrudolph commented 9 years ago

I just found out that the script really was broken and it just worked for me before only because of an unclean work directory. I fixed it in the launcher script (adding the out directory) as you suggested and added extra diagnostic output to the Java attacher in case the library is missing.

Thanks for raising this issue.