Benchmarks that runs server and client and separate processes. #1952
Conversation
MakMukhi
force-pushed
the
bench
branch
2 times, most recently
from
6e47fae to
ede983e
Compare
benchmark/run_bench.sh
Outdated
| done | ||
|
|
||
| # Build server and client | ||
| go install $GOPATH/src/google.golang.org/grpc/benchmark/server/benchserver.go && go install $GOPATH/src/google.golang.org/grpc/benchmark/client/benchclient.go |
There was a problem hiding this comment.
Is it possible to do this with -o benchserver and avoid renaming the files?
benchmark/client/main.go
Outdated
| trace = flag.Bool("trace", true, "Whether tracing is on") | ||
| rpcType = flag.Int("rpc_type", 0, | ||
| port = flag.String("port", "50051", "Localhost port to connect to.") | ||
| r = flag.Int("r", 1, "The number of concurrent RPCs on each connection.") |
There was a problem hiding this comment.
These guys are global variables, and as such, should have long names. Rationale:
- Used far away from where defined.
- Shadowed by commonly-used locals.
benchmark/run_bench.sh
Outdated
| @@ -0,0 +1,207 @@ | |||
| #!/bin/bash | |||
|
|
|||
| rs=(1) | |||
There was a problem hiding this comment.
Same comment about names -- for readability, these should be descriptive.
benchmark/run_bench.sh
Outdated
| resps=(1) | ||
| rpc_types=(unary) | ||
|
|
||
| # idx[0] = idx value for rs |
There was a problem hiding this comment.
All input parameters that are arrays, need an index to go over them while keeping other parameters' index unchanged. This way we can generate all combinations without using nested for loops
benchmark/run_bench.sh
Outdated
| fi | ||
| done | ||
| if [ ${return} == 1 ]; then | ||
| rm -Rf ${out_dir} |
benchmark/run_bench.sh
Outdated
|
|
||
| inc() | ||
| { | ||
| for i in $(seq $((${#idx[@]}-1)) -1 0) |
There was a problem hiding this comment.
I prefer:
for _____; do
which you use below. (Let's be consistent at the very least.)
benchmark/run_bench.sh
Outdated
| case "$1" in | ||
| -r) | ||
| shift | ||
| if test $# -gt 0; then |
There was a problem hiding this comment.
prefer:
if [ $# -eq 0 ]; then # or [[ $# == 0 ]]
echo "error stuff"
exit 1
fi
rs=....
...
benchmark/run_bench.sh
Outdated
| idx_max[4]=${#rpc_types[@]} | ||
| for val in ${rpc_types[@]}; do | ||
| case "${val}" in | ||
| "unary"|"streaming");; |
There was a problem hiding this comment.
do we need to vet the vals here, or can we pass them to the script and wait for it to fail?
benchmark/run_bench.sh
Outdated
| ;; | ||
| -h|--help) | ||
| echo "Following are valid options:" | ||
| echo "" |
benchmark/run_bench.sh
Outdated
| ;; | ||
| -c) | ||
| shift | ||
| if test $# -gt 0; then |
There was a problem hiding this comment.
Consider a refactor:
param() {
argname=$1
shift
if [ $# -eq 0 ]; then
echo "$argname not specified"
exit 1
fi
echo $1 | sed 's/,/ /g'
}
...
-c)
shift
cs=$(param "number of connections" "$@")
shift
There was a problem hiding this comment.
cs=$(param "arg1" "arg2") doesn't call param(). Maybe, I'm doing something wrong.
There was a problem hiding this comment.
Maybe so?
$ x() { echo "hi"; }
$ y=$(x)
$ echo $y
hi
benchmark/run_bench.sh
Outdated
| -r) | ||
| shift | ||
| if test $# -gt 0; then | ||
| rs=($(echo $1 | sed 's/,/ /g')) |
There was a problem hiding this comment.
why a subshell?
This is preferred: rs="$(echo "$1" | sed 's/,/ /g')"
There was a problem hiding this comment.
rs is an array. rs=(1 2 3)
There was a problem hiding this comment.
Oh yes, I see...parens are overloaded.
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| -r) | ||
| shift |
There was a problem hiding this comment.
You can (re)move these shifts:
while ... do
case "$1" in
-r)
set_param "number of rpcs" 0 $2
rpcs=...
;;
...
esac
shift; shift
done
Also,
Sample output: