2
\$\begingroup\$

This is my first serious shell script, so it is probably horrible.

Problem statement

I decided to improve my profiling skills as so far they were mostly about hand-rolling counters and timers. After trying out better profiling tools and techniques I noticed that previously I was just rolling my face over keyboard until the metrics got better. Yet to do profiling in a consistent and hassle-free way is a bit tedious.

perf is a profiling utility used to collect software and hardware metrics about program's execution or entirety of the machine. It outputs binary files that can be fed into variety of tools, but I decided on FlameGraph. The problem with it is that it takes several commands and the overall process is rather boring. I decided to automate it with a shell script that invokes needed commands with the flags I usually use.

Code

#!/bin/sh

usage="$(basename "$0") [-g fgdir] [-h] -t target

where:
    -h  show this help text
    -t  executable to run under perf
    -g  root directory of FlameGraph repository https://github.com/brendangregg/FlameGraph
"

fgroot="."
while getopts 'g:ht:' option; do
    case "$option" in
      h) echo "$usage"
         exit
         ;;
      t) if ! [ -x "$(command -v "$OPTARG")" ]; then
            echo "$OPTARG is not executable"
            exit 1
         fi
         echo "$OPTARG"
         target="$OPTARG"
         ;;
      g) fgroot="$OPTARG"
         echo "$OPTARG"
         ;;
      :) printf "missing argument for -%s\n" "$OPTARG" >&2
         echo "$usage" >&2
         exit 1
         ;;
     \?) printf "illegal option: -%s\n" "$OPTARG" >&2
         echo "$usage" >&2
         exit 1
         ;;
    esac
done
shift $((OPTIND - 1))

scpath="$fgroot/stackcollapse-perf.pl"
fgpath="$fgroot/flamegraph.pl"
echo "$scpath" "$fgpath"
if ! [ "$(command -v "$scpath")" ]; then
  printf "could not find stackcollapse-perf.pl\n"
  exit 1
fi

if ! [ "$(command -v "$fgpath")" ]; then
  printf "could not find flamegraph.pl\n"
  exit 1
fi


#filename=$(date +"%Y-%m-%d-%Hh-%Mm-%Ss").perf
filename=$(date -u +"%Y-%m-%dT%H:%M:%S%Z")
perf record -g --call-graph=dwarf -F 99 "$target"
perf script > "$filename".perf
$scpath "$filename".perf > "$filename".collapsed
$fgpath "$filename".collapsed > "$filename".svg


Is the interface idiomatic (command line argument letters, usage string, etc)? Is it possible to write the shell script in an easier to understand way? Is there any more useful checking I should do inside my code?


Sample output:

enter image description here

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

Some of your diagnostic messages are being sent to stdout; you want to send all of them to stderr (>&2). A good practice is to include the script's name in these messages, so you can see who is emitting a message when you have scripts calling scripts calling scripts etc.

(Many of these echos were apparently just for debugging anyway; I commented out those instead.)

As a minor refactoring, I turned usage into a function.

I made usage return exit code 1, which is a fairly common convention, and thus changed the other error cases to return exit codes 2, 3, etc to allow a caller to distinguish between these cases.

Perhaps make up your mind regarding whether to use echo or printf. The former is less versatile but often more compact for simple messages.

The check for whether the argument to -t is executable or not is slightly problematic; command -v will not output anything if it isn't. But I don't see an easy way to rephrase that. Perhaps the error message should be rearticulated, though?

The conventional way to use command -v is to simply run it directly, without a command substitution; perhaps see also Why is testing "$?" to see if a command succeeded or not, an anti-pattern? for a discussion, though that's not exactly the problem here.

#!/bin/sh

me="$(basename "$0")"

usage () {
    printf '%s\n' >&2 \
    "$me [-g fgdir] [-h] -t target" "" \
    "    where:" \
    "    -h  show this help text" \
    "    -t  executable to run under perf" \
    "    -g  root directory of FlameGraph repository https://github.com/brendangregg/FlameGraph" ""
    exit 1
}


fgroot="."
while getopts 'g:ht:' option; do
    case "$option" in
      h) usage;;
      t) if ! [ -x "$(command -v "$OPTARG")" ]; then
            echo "$me: $OPTARG is not executable" >&2
            exit 2
         fi
         # echo "$OPTARG"
         target="$OPTARG"
         ;;
      g) fgroot="$OPTARG"
         # echo "$OPTARG"
         ;;
      :) printf "%s: missing argument for -%s\n" "$me" "$OPTARG" >&2
         usage
         ;;
     \?) printf "%s: illegal option: -%s\n" "$me" "$OPTARG" >&2
         usage
         ;;
    esac
done
shift $((OPTIND - 1))

scpath="$fgroot/stackcollapse-perf.pl"
fgpath="$fgroot/flamegraph.pl"
# echo "$scpath" "$fgpath"
if ! command -v "$scpath" >/dev/null; then
  printf "%s: could not find stackcollapse-perf.pl\n" "$me" >&2
  exit 3
fi

if ! command -v "$fgpath" >/dev/null; then
  printf "%s: could not find flamegraph.pl\n" "$me" >&2
  exit 4
fi

filename=$(date -u +"%Y-%m-%dT%H:%M:%S%Z")
perf record -g --call-graph=dwarf -F 99 "$target"
perf script > "$filename".perf
$scpath "$filename".perf > "$filename".collapsed
$fgpath "$filename".collapsed > "$filename".svg

Options should arguably be optional; but I refrained from removing the -t option in case you need it to stay for reasons of backwards compatibility.

If stackcollapse-perf.pl accepts input from stdin, you could avoid creating the .collapsed file. These temporary files in the current directory which can get overwritten at any time are still a bit of a wart.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.