From 4166dc3536986c5295131a6e7c5920baaae487b3 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Thu, 9 Mar 2017 14:30:18 -0800 Subject: [PATCH 1/5] Add script to allow Jenkins to comment on PRs --- tools/jenkins/comment_on_pr.sh | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 tools/jenkins/comment_on_pr.sh diff --git a/tools/jenkins/comment_on_pr.sh b/tools/jenkins/comment_on_pr.sh new file mode 100644 index 00000000000..79c54c2159c --- /dev/null +++ b/tools/jenkins/comment_on_pr.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash +# Copyright 2017, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# This script is invoked by Jenkins to comment $1 on pull requests +# when triggered by a build + +set -e + +if [ -z $1 ] || [ -z $JENKINS_OAUTH_TOKEN ] || [ -z $ghprbPullId ]; then + echo "Insufficient arguments or environment variables provided." + exit 1 +fi + +# Format the comment message to JSON +COMMENT_MESSAGE="{\"body\":\"$1\"}" + +curl -k -H "Authorization: token $JENKINS_OAUTH_TOKEN" -H "Content-Type: application/json" \ + -d "$COMMENT_MESSAGE" https://api.github.com/repos/grpc/grpc/issues/$ghprbPullId/comments From aff1c05ed78e52f41c8b1e7c03b3c57f08368130 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Thu, 9 Mar 2017 15:08:01 -0800 Subject: [PATCH 2/5] Make Jenkins post microbenchmarking diff in a comment --- tools/jenkins/comment_on_pr.sh | 2 +- tools/run_tests/run_microbenchmark.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) mode change 100644 => 100755 tools/jenkins/comment_on_pr.sh diff --git a/tools/jenkins/comment_on_pr.sh b/tools/jenkins/comment_on_pr.sh old mode 100644 new mode 100755 index 79c54c2159c..85f33aa917a --- a/tools/jenkins/comment_on_pr.sh +++ b/tools/jenkins/comment_on_pr.sh @@ -33,7 +33,7 @@ set -e -if [ -z $1 ] || [ -z $JENKINS_OAUTH_TOKEN ] || [ -z $ghprbPullId ]; then +if [ -z "$1" ] || [ -z $JENKINS_OAUTH_TOKEN ] || [ -z $ghprbPullId ]; then echo "Insufficient arguments or environment variables provided." exit 1 fi diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index 57b2636e569..12d98158a0e 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -229,6 +229,8 @@ argp.add_argument('--summary_time', type=int, help='Minimum time to run benchmarks for the summary collection') args = argp.parse_args() +if args.diff_perf: + git_comment = '' try: for collect in args.collect: @@ -262,7 +264,14 @@ try: if diff: heading('Performance diff: %s' % bm_name) text(diff) + git_comment += '```\\nPerformance diff: %s\\n%s\\n```\\n' % (bm_name, diff.replace('\n', '\\n')) finally: + if args.diff_perf: + subprocess.call(['tools/jenkins/comment_on_pr.sh "%s"' % git_comment.replace('`', '\`')], + stdout=subprocess.PIPE, + shell=True) + if not os.path.exists('reports'): + os.makedirs('reports') index_html += "\n\n" with open('reports/index.html', 'w') as f: f.write(index_html) From d0ee10df9bc80d03d8cb96db71440284980d8889 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Fri, 10 Mar 2017 22:37:52 -0800 Subject: [PATCH 3/5] Change jenkins/run_performance.sh to use microbenchmarking --- tools/jenkins/run_performance.sh | 7 ++++++- tools/run_tests/run_microbenchmark.py | 30 ++++++++++++++------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/tools/jenkins/run_performance.sh b/tools/jenkins/run_performance.sh index 99b920f6a09..bc48f1eeb62 100755 --- a/tools/jenkins/run_performance.sh +++ b/tools/jenkins/run_performance.sh @@ -31,7 +31,12 @@ # This script is invoked by Jenkins and runs performance smoke test. set -ex +# List of benchmarks that provide good signal for analyzing performance changes in pull requests +BENCHMARKS_TO_RUN="bm_closure bm_cq bm_call_create bm_error bm_chttp2_hpack bm_metadata" + # Enter the gRPC repo root cd $(dirname $0)/../.. -tools/run_tests/run_performance_tests.py -l c++ node ruby csharp python --netperf --category smoketest +# tools/run_tests/run_performance_tests.py -l c++ node ruby csharp python --netperf --category smoketest +# todo(mattkwong): Change performance test to use microbenchmarking +tools/run_tests/run_microbenchmark.py -c summary --diff_perf origin/$ghprbTargetBranch -b $BENCHMARKS_TO_RUN diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index 12d98158a0e..de0c7176d48 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -38,6 +38,17 @@ import argparse import python_utils.jobset as jobset import python_utils.start_port_server as start_port_server +_AVAILABLE_BENCHMARK_TESTS = ['bm_fullstack_unary_ping_pong', + 'bm_fullstack_streaming_ping_pong', + 'bm_fullstack_streaming_pump', + 'bm_closure', + 'bm_cq', + 'bm_call_create', + 'bm_error', + 'bm_chttp2_hpack', + 'bm_metadata', + 'bm_fullstack_trickle'] + flamegraph_dir = os.path.join(os.path.expanduser('~'), 'FlameGraph') os.chdir(os.path.join(os.path.dirname(sys.argv[0]), '../..')) @@ -201,17 +212,8 @@ argp.add_argument('-c', '--collect', default=sorted(collectors.keys()), help='Which collectors should be run against each benchmark') argp.add_argument('-b', '--benchmarks', - default=['bm_fullstack_unary_ping_pong', - 'bm_fullstack_streaming_ping_pong', - 'bm_fullstack_streaming_pump', - 'bm_closure', - 'bm_cq', - 'bm_call_create', - 'bm_error', - 'bm_chttp2_hpack', - 'bm_metadata', - 'bm_fullstack_trickle', - ], + choices=_AVAILABLE_BENCHMARK_TESTS, + default=_AVAILABLE_BENCHMARK_TESTS, nargs='+', type=str, help='Which microbenchmarks should be run') @@ -229,20 +231,20 @@ argp.add_argument('--summary_time', type=int, help='Minimum time to run benchmarks for the summary collection') args = argp.parse_args() -if args.diff_perf: - git_comment = '' try: for collect in args.collect: for bm_name in args.benchmarks: collectors[collect](bm_name, args) if args.diff_perf: + git_comment = 'Performance differences between this PR and %s\\n' % args.diff_perf if 'summary' not in args.collect: for bm_name in args.benchmarks: run_summary(bm_name, 'opt', bm_name) run_summary(bm_name, 'counters', bm_name) where_am_i = subprocess.check_output(['git', 'rev-parse', '--abbrev-ref', 'HEAD']).strip() - subprocess.check_call(['git', 'checkout', args.diff_perf]) + # todo(mattkwong): uncomment this before merging + # subprocess.check_call(['git', 'checkout', args.diff_perf]) comparables = [] subprocess.check_call(['make', 'clean']) try: From 063adf3fb4f6ed8bb342bfe2c8086e1db9356aa1 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Wed, 22 Mar 2017 12:48:34 -0700 Subject: [PATCH 4/5] Add additional benchmarks --- tools/jenkins/run_performance.sh | 2 +- tools/run_tests/run_microbenchmark.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/jenkins/run_performance.sh b/tools/jenkins/run_performance.sh index bc48f1eeb62..c3be3082d49 100755 --- a/tools/jenkins/run_performance.sh +++ b/tools/jenkins/run_performance.sh @@ -32,7 +32,7 @@ set -ex # List of benchmarks that provide good signal for analyzing performance changes in pull requests -BENCHMARKS_TO_RUN="bm_closure bm_cq bm_call_create bm_error bm_chttp2_hpack bm_metadata" +BENCHMARKS_TO_RUN="bm_closure bm_cq bm_call_create bm_error bm_chttp2_hpack bm_chttp2_transport bm_pollset bm_metadata" # Enter the gRPC repo root cd $(dirname $0)/../.. diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index de0c7176d48..8b9170edf41 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -46,6 +46,8 @@ _AVAILABLE_BENCHMARK_TESTS = ['bm_fullstack_unary_ping_pong', 'bm_call_create', 'bm_error', 'bm_chttp2_hpack', + 'bm_chttp2_transport', + 'bm_pollset', 'bm_metadata', 'bm_fullstack_trickle'] From 504adae3722d6ea9f952f8990be16e5490fa1a6b Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Fri, 24 Mar 2017 17:01:27 -0700 Subject: [PATCH 5/5] Add a TODO to enable microbenmarking for performance testing --- tools/jenkins/run_performance.sh | 4 ++-- tools/run_tests/run_microbenchmark.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/jenkins/run_performance.sh b/tools/jenkins/run_performance.sh index c3be3082d49..2b05a8d04d0 100755 --- a/tools/jenkins/run_performance.sh +++ b/tools/jenkins/run_performance.sh @@ -37,6 +37,6 @@ BENCHMARKS_TO_RUN="bm_closure bm_cq bm_call_create bm_error bm_chttp2_hpack bm_c # Enter the gRPC repo root cd $(dirname $0)/../.. -# tools/run_tests/run_performance_tests.py -l c++ node ruby csharp python --netperf --category smoketest +tools/run_tests/run_performance_tests.py -l c++ node ruby csharp python --netperf --category smoketest # todo(mattkwong): Change performance test to use microbenchmarking -tools/run_tests/run_microbenchmark.py -c summary --diff_perf origin/$ghprbTargetBranch -b $BENCHMARKS_TO_RUN +# tools/run_tests/run_microbenchmark.py -c summary --diff_perf origin/$ghprbTargetBranch -b $BENCHMARKS_TO_RUN diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index 8b9170edf41..19cf16ed499 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -245,8 +245,7 @@ try: run_summary(bm_name, 'opt', bm_name) run_summary(bm_name, 'counters', bm_name) where_am_i = subprocess.check_output(['git', 'rev-parse', '--abbrev-ref', 'HEAD']).strip() - # todo(mattkwong): uncomment this before merging - # subprocess.check_call(['git', 'checkout', args.diff_perf]) + subprocess.check_call(['git', 'checkout', args.diff_perf]) comparables = [] subprocess.check_call(['make', 'clean']) try: