{"thread":{"id":"48240","subject":"[PATCH v1 0/2] t/perf: bisect performance regressions","startedAt":"2018-04-08T09:35:35Z","lastAt":"2018-04-08T09:45:06Z","messageCount":4,"participants":["Christian Couder","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"344089","messageId":"20180408093513.17769-1-chriscool@tuxfamily.org","threadId":"48240","inReplyTo":null,"subject":"[PATCH v1 0/2] t/perf: bisect performance regressions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-04-08T09:35:11Z","receivedAt":"2018-04-08T09:35:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is a small patch series on top of cc/perf-aggregate-sort, which\nis next, to add scripts that make it much easier to bisect performance\nregressions.\n\nFor example if you ran perf test using the perf.conf config file that\ncontains a \"with libpcre\", then using:\n\n$ ./aggregate.perl --subsection \"with libpcre\" --sort-by=regression v2.15.1 v2.16.2 v2.17.0 \n\nyou can get a line in the output like:\n\n+8.2% p5302-pack-index.6 14.33(44.50+0.77) 15.50(43.83+0.94) v2.16.2 v2.17.0\n\nand you can now use this output line to bisect where the regression\ncomes from using:\n\necho \"+8.2% p5302-pack-index.6 14.33(44.50+0.77) 15.50(43.83+0.94) v2.16.2 v2.17.0\" | ./bisect_regression --config perf.conf --subsection \"with libpcre\"\n\nCaveats:\n\nYou must make sure that the times are consistent between runs and that\nthere is a significant time difference between the \"good\" runs and the\n\"bad\" runs.\n\nFor example the following is not easily bisectable:\n\n+100.0% p0000-perf-lib-sanity.2 0.01(0.01+0.01) 0.02(0.02+0.00) v2.15.1 v2.16.2\n\nas the rtime went from 0.01 s to 0.02 s and we only have 0.01 s precision.\n\nChristian Couder (2):\n  perf/run: add --subsection option\n  t/perf: add scripts to bisect performance regressions\n\n t/perf/bisect_regression | 73 ++++++++++++++++++++++++++++++++++++++++\n t/perf/bisect_run_script | 47 ++++++++++++++++++++++++++\n t/perf/run               | 56 ++++++++++++++++++++++++------\n 3 files changed, 166 insertions(+), 10 deletions(-)\n create mode 100755 t/perf/bisect_regression\n create mode 100755 t/perf/bisect_run_script\n\n-- \n2.17.0.rc1.36.g098d832c9.dirty\n\n"},{"id":"344090","messageId":"20180408093513.17769-2-chriscool@tuxfamily.org","threadId":"48240","inReplyTo":"20180408093513.17769-1-chriscool@tuxfamily.org","subject":"[PATCH v1 1/2] perf/run: add --subsection option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-04-08T09:35:12Z","receivedAt":"2018-04-08T09:35:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new option makes it possible to run perf tests as defined\nin only one subsection of a config file.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/run | 56 ++++++++++++++++++++++++++++++++++++++++++++----------\n 1 file changed, 46 insertions(+), 10 deletions(-)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex 213da5d6b9..9aaa733c77 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -1,21 +1,34 @@\n #!/bin/sh\n \n-case \"$1\" in\n+die () {\n+\techo >&2 \"error: $*\"\n+\texit 1\n+}\n+\n+while [ $# -gt 0 ]; do\n+\targ=\"$1\"\n+\tcase \"$arg\" in\n+\t--)\n+\t\tbreak ;;\n \t--help)\n-\t\techo \"usage: $0 [--config file] [other_git_tree...] [--] [test_scripts]\"\n-\t\texit 0\n-\t\t;;\n+\t\techo \"usage: $0 [--config file] [--subsection subsec] [other_git_tree...] [--] [test_scripts]\"\n+\t\texit 0 ;;\n \t--config)\n \t\tshift\n \t\tGIT_PERF_CONFIG_FILE=$(cd \"$(dirname \"$1\")\"; pwd)/$(basename \"$1\")\n \t\texport GIT_PERF_CONFIG_FILE\n \t\tshift ;;\n-esac\n-\n-die () {\n-\techo >&2 \"error: $*\"\n-\texit 1\n-}\n+\t--subsection)\n+\t\tshift\n+\t\tGIT_PERF_SUBSECTION=\"$1\"\n+\t\texport GIT_PERF_SUBSECTION\n+\t\tshift ;;\n+\t--*)\n+\t\tdie \"unrecognised option: '$arg'\" ;;\n+\t*)\n+\t\tbreak ;;\n+\tesac\n+done\n \n run_one_dir () {\n \tif test $# -eq 0; then\n@@ -172,9 +185,32 @@ get_subsections \"perf\" >test-results/run_subsections.names\n \n if test $(wc -l <test-results/run_subsections.names) -eq 0\n then\n+\tif test -n \"$GIT_PERF_SUBSECTION\"\n+\tthen\n+\t\tif test -n \"$GIT_PERF_CONFIG_FILE\"\n+\t\tthen\n+\t\t\tdie \"no subsections are defined in config file '$GIT_PERF_CONFIG_FILE'\"\n+\t\telse\n+\t\t\tdie \"subsection '$GIT_PERF_SUBSECTION' defined without a config file\"\n+\t\tfi\n+\tfi\n \t(\n \t\trun_subsection \"$@\"\n \t)\n+elif test -n \"$GIT_PERF_SUBSECTION\"\n+then\n+\tegrep \"^$GIT_PERF_SUBSECTION\\$\" test-results/run_subsections.names >/dev/null ||\n+\t\tdie \"subsection '$GIT_PERF_SUBSECTION' not found in '$GIT_PERF_CONFIG_FILE'\"\n+\n+\tegrep \"^$GIT_PERF_SUBSECTION\\$\" test-results/run_subsections.names | while read -r subsec\n+\tdo\n+\t\t(\n+\t\t\tGIT_PERF_SUBSECTION=\"$subsec\"\n+\t\t\texport GIT_PERF_SUBSECTION\n+\t\t\techo \"======== Run for subsection '$GIT_PERF_SUBSECTION' ========\"\n+\t\t\trun_subsection \"$@\"\n+\t\t)\n+\tdone\n else\n \twhile read -r subsec\n \tdo\n-- \n2.17.0.rc1.36.g098d832c9.dirty\n\n"},{"id":"344091","messageId":"20180408093513.17769-3-chriscool@tuxfamily.org","threadId":"48240","inReplyTo":"20180408093513.17769-1-chriscool@tuxfamily.org","subject":"[PATCH v1 2/2] t/perf: add scripts to bisect performance regressions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-04-08T09:35:13Z","receivedAt":"2018-04-08T09:35:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The new bisect_regression script can be used to automatically bisect\nperformance regressions. It will pass the new bisect_run_script to\n`git bisect run`.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/bisect_regression | 73 ++++++++++++++++++++++++++++++++++++++++\n t/perf/bisect_run_script | 47 ++++++++++++++++++++++++++\n 2 files changed, 120 insertions(+)\n create mode 100755 t/perf/bisect_regression\n create mode 100755 t/perf/bisect_run_script\n\ndiff --git a/t/perf/bisect_regression b/t/perf/bisect_regression\nnew file mode 100755\nindex 0000000000..a94d9955d0\n--- /dev/null\n+++ b/t/perf/bisect_regression\n@@ -0,0 +1,73 @@\n+#!/bin/sh\n+\n+# Read a line coming from `./aggregate.perl --sort-by regression ...`\n+# and automatically bisect to find the commit responsible for the\n+# performance regression.\n+#\n+# Lines from `./aggregate.perl --sort-by regression ...` look like:\n+#\n+# +100.0% p7821-grep-engines-fixed.1 0.04(0.10+0.03) 0.08(0.11+0.08) v2.14.3 v2.15.1\n+# +33.3% p7820-grep-engines.1 0.03(0.08+0.02) 0.04(0.08+0.02) v2.14.3 v2.15.1\n+#\n+\n+die () {\n+\techo >&2 \"error: $*\"\n+\texit 1\n+}\n+\n+while [ $# -gt 0 ]; do\n+\targ=\"$1\"\n+\tcase \"$arg\" in\n+\t--help)\n+\t\techo \"usage: $0 [--config file] [--subsection subsection]\"\n+\t\texit 0\n+\t\t;;\n+\t--config)\n+\t\tshift\n+\t\tGIT_PERF_CONFIG_FILE=$(cd \"$(dirname \"$1\")\"; pwd)/$(basename \"$1\")\n+\t\texport GIT_PERF_CONFIG_FILE\n+\t\tshift ;;\n+\t--subsection)\n+\t\tshift\n+\t\tGIT_PERF_SUBSECTION=\"$1\"\n+\t\texport GIT_PERF_SUBSECTION\n+\t\tshift ;;\n+\t--*)\n+\t\tdie \"unrecognised option: '$arg'\" ;;\n+\t*)\n+\t\tdie \"unknown argument '$arg'\"\n+\t\t;;\n+\tesac\n+done\n+\n+read -r regression subtest oldtime newtime oldrev newrev\n+\n+test_script=$(echo \"$subtest\" | sed -e 's/\\(.*\\)\\.[0-9]*$/\\1.sh/')\n+test_number=$(echo \"$subtest\" | sed -e 's/.*\\.\\([0-9]*\\)$/\\1/')\n+\n+# oldtime and newtime are decimal number, not integers\n+\n+oldtime=$(echo \"$oldtime\" | sed -e 's/^\\([0-9]\\+\\.[0-9]\\+\\).*$/\\1/')\n+newtime=$(echo \"$newtime\" | sed -e 's/^\\([0-9]\\+\\.[0-9]\\+\\).*$/\\1/')\n+\n+test $(echo \"$newtime\" \"$oldtime\" | awk '{ print ($1 > $2) }') = 1 ||\n+\tdie \"New time '$newtime' shoud be greater than old time '$oldtime'\"\n+\n+tmpdir=$(mktemp -d -t bisect_regression_XXXXXX) || die \"Failed to create temp directory\"\n+echo \"$oldtime\" >\"$tmpdir/oldtime\" || die \"Failed to write to '$tmpdir/oldtime'\"\n+echo \"$newtime\" >\"$tmpdir/newtime\" || die \"Failed to write to '$tmpdir/newtime'\"\n+\n+# Bisecting must be performed from the top level directory (even with --no-checkout)\n+(\n+\ttoplevel_dir=$(git rev-parse --show-toplevel) || die \"Failed to find top level directory\"\n+\tcd \"$toplevel_dir\" || die \"Failed to cd into top level directory '$toplevel_dir'\"\n+\n+\tgit bisect start --no-checkout \"$newrev\" \"$oldrev\" || die \"Failed to start bisecting\"\n+\n+\tgit bisect run t/perf/bisect_run_script \"$test_script\" \"$test_number\" \"$tmpdir\"\n+\tres=\"$?\"\n+\n+\tgit bisect reset\n+\n+\texit \"$res\"\n+)\ndiff --git a/t/perf/bisect_run_script b/t/perf/bisect_run_script\nnew file mode 100755\nindex 0000000000..038255df4b\n--- /dev/null\n+++ b/t/perf/bisect_run_script\n@@ -0,0 +1,47 @@\n+#!/bin/sh\n+\n+script=\"$1\"\n+test_number=\"$2\"\n+info_dir=\"$3\"\n+\n+# This aborts the bisection immediately\n+die () {\n+\techo >&2 \"error: $*\"\n+\texit 255\n+}\n+\n+bisect_head=$(git rev-parse --verify BISECT_HEAD) || die \"Failed to find BISECT_HEAD ref\"\n+\n+script_number=$(echo \"$script\" | sed -e \"s/^p\\([0-9]*\\).*\\$/\\1/\") || die \"Failed to get script number for '$script'\"\n+\n+oldtime=$(cat \"$info_dir/oldtime\") || die \"Failed to access '$info_dir/oldtime'\"\n+newtime=$(cat \"$info_dir/newtime\") || die \"Failed to access '$info_dir/newtime'\"\n+\n+cd t/perf || die \"Failed to cd into 't/perf'\"\n+\n+result_file=\"$info_dir/perf_${script_number}_${bisect_head}_results.txt\"\n+\n+GIT_PERF_DIRS_OR_REVS=\"$bisect_head\"\n+export GIT_PERF_DIRS_OR_REVS\n+\n+./run \"$script\" >\"$result_file\" 2>&1 || die \"Failed to run perf test '$script'\"\n+\n+rtime=$(sed -n \"s/^$script_number\\.$test_number:.*\\([0-9]\\+\\.[0-9]\\+\\)(.*).*\\$/\\1/p\" \"$result_file\")\n+\n+echo \"newtime: $newtime\"\n+echo \"rtime: $rtime\"\n+echo \"oldtime: $oldtime\"\n+\n+# Compare ($newtime - $rtime) with ($rtime - $oldtime)\n+# Times are decimal number, not integers\n+\n+if test $(echo \"$newtime\" \"$rtime\" \"$oldtime\" | awk '{ print ($1 - $2 > $2 - $3) }') = 1\n+then\n+\t# Current commit is considered \"good/old\"\n+\techo \"$rtime\" >\"$info_dir/oldtime\"\n+\texit 0\n+else\n+\t# Current commit is considered \"bad/new\"\n+\techo \"$rtime\" >\"$info_dir/newtime\"\n+\texit 1\n+fi\n-- \n2.17.0.rc1.36.g098d832c9.dirty\n\n"},{"id":"344092","messageId":"CAPig+cT=kn0hujoeLxkYTv1wZA93QxP-CQPepmzs1SDq1QJWEw@mail.gmail.com","threadId":"48240","inReplyTo":"20180408093513.17769-2-chriscool@tuxfamily.org","subject":"Re: [PATCH v1 1/2] perf/run: add --subsection option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-08T09:44:58Z","receivedAt":"2018-04-08T09:45:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 8, 2018 at 5:35 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> This new option makes it possible to run perf tests as defined\n> in only one subsection of a config file.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n> diff --git a/t/perf/run b/t/perf/run\n> @@ -1,21 +1,34 @@\n> +while [ $# -gt 0 ]; do\n\nI was going to ask if you meant to use 'test' here rather than '[',\nhowever, I see that this script already has a mix of the two, so...\n\nLikewise, scripts in Git usually omit the semicolon and place 'do' on\nits own line, however, again I see that this script has a mix of the\ntwo styles, so...\n"}]}