# [PATCH v3 1/7] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION} 8 messages from 2018-01-05 to 2018-01-05. Participants: Christian Couder. Thread: https://gitlist.dev/t/47542 ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 1/7] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION} Message-ID: <20180105091226.16083-2-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-2-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` The way we check ENV{GIT_PERF_SUBSECTION} could trigger comparison between undef and "" that may be flagged by use of strict & warnings. Let's fix that. Signed-off-by: Christian Couder --- t/perf/aggregate.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index e401208488..769d418708 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -70,7 +70,7 @@ if (not @tests) { } my $resultsdir = "test-results"; -if ($ENV{GIT_PERF_SUBSECTION} ne "") { +if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") { $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION}; } -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 0/7] Codespeed perf results Message-ID: <20180105091226.16083-1-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-1-chriscool%40tuxfamily.org ``` This patch series is built on top of cc/perf-run-config which recently graduated to master. It makes it possible to send perf results to a Codespeed server. See https://github.com/tobami/codespeed/ and web sites like http://speed.pypy.org/ which are using Codespeed. The end goal would be to have such a server always available to track how the different git commands perform over time on different kind of repos (small, medium, large, ...) with different optimizations on and off (split-index, libpcre2, BLK_SHA1, ...) With this series and a config file like: $ cat perf.conf [perf] dirsOrRevs = v2.12.0 v2.13.0 repeatCount = 10 sendToCodespeed = http://localhost:8000 repoName = Git repo [perf "with libpcre"] makeOpts = "DEVELOPER=1 USE_LIBPCRE=YesPlease" [perf "without libpcre"] makeOpts = "DEVELOPER=1" One should be able to just launch: $ ./run --config perf.conf p7810-grep.sh and then get nice graphs in a Codespeed instance running on http://localhost:8000. Caveat ~~~~~~ For now one has to create the "Git repo" environment (in fact all the values of the "environment" field sent to Codespeed) in the Codespeed admin interface. (We send the perf.repoName config variable in the "environment" Codespeed field.) This is because Codespeed requires the environment fields to be created and does not provide a simple way to create these fields programmatically. There are discussions on a Codespeed issue (https://github.com/tobami/codespeed/issues/232) about creating a proper API for Codespeed that could address this problem in the future. Changes since previous version ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ There are very few changes compared to v2: - In patch 1/7 commit message has been improved following a comment by Junio. - In patch 3/7 some debugging comments were removed and 'use JSON;' was moved to the top of aggregate.perl as suggested by Junio. The diff is the following: diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 34d74fc015..5c439f6bc2 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -3,6 +3,7 @@ use lib '../../perl/blib/lib'; use strict; use warnings; +use JSON; use Git; sub get_times { @@ -226,10 +227,6 @@ sub print_codespeed_results { } } - #use Data::Dumper qw/ Dumper /; - #print Dumper(\@data); - - use JSON; print to_json(\@data, {utf8 => 1, pretty => 1}), "\n"; } Links ~~~~~ This patch series: https://github.com/chriscool/git/commits/codespeed Previous versions: v1: https://github.com/chriscool/git/commits/codespeed1 v2: https://github.com/chriscool/git/commits/codespeed10 Discussions: v1: https://public-inbox.org/git/CAP8UFD3Q4h-aYBDABSPOW948LQYVydWZ1hLPAD+kr9ZpXVZiaQ@mail.gmail.com/ v2: https://public-inbox.org/git/20171226215908.425-1-chriscool@tuxfamily.org/ Discussions about the cc/perf-run-config patch series: v1: https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/ v2: https://public-inbox.org/git/CAP8UFD2j-UFh+9awz91gtZ-jusq7EUOExMgURO59vpf29jXS4A@mail.gmail.com/ Christian Couder (7): perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION} perf/aggregate: refactor printing results perf/aggregate: implement codespeed JSON output perf/run: add conf_opts argument to get_var_from_env_or_config() perf/run: learn about perf.codespeedOutput perf/run: learn to send output to codespeed server perf/run: read GIT_PERF_REPO_NAME from perf.repoName t/perf/aggregate.perl | 160 +++++++++++++++++++++++++++++++++++--------------- t/perf/run | 31 ++++++++-- 2 files changed, 137 insertions(+), 54 deletions(-) -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 2/7] perf/aggregate: refactor printing results Message-ID: <20180105091226.16083-3-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-3-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` As we want to implement another kind of output than the current output for the perf test results, let's refactor the existing code that outputs the results in its own print_default_results() function. Signed-off-by: Christian Couder --- t/perf/aggregate.perl | 96 +++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 46 deletions(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 769d418708..3609cb5dc3 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -100,13 +100,6 @@ sub read_descr { return $line; } -my %descrs; -my $descrlen = 4; # "Test" -for my $t (@subtests) { - $descrs{$t} = $shorttests{$t}.": ".read_descr("$resultsdir/$t.descr"); - $descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen; -} - sub have_duplicate { my %seen; for (@_) { @@ -122,54 +115,65 @@ sub have_slash { return 0; } -my %newdirabbrevs = %dirabbrevs; -while (!have_duplicate(values %newdirabbrevs)) { - %dirabbrevs = %newdirabbrevs; - last if !have_slash(values %dirabbrevs); - %newdirabbrevs = %dirabbrevs; - for (values %newdirabbrevs) { - s{^[^/]*/}{}; +sub print_default_results { + my %descrs; + my $descrlen = 4; # "Test" + for my $t (@subtests) { + $descrs{$t} = $shorttests{$t}.": ".read_descr("$resultsdir/$t.descr"); + $descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen; } -} -my %times; -my @colwidth = ((0)x@dirs); -for my $i (0..$#dirs) { - my $d = $dirs[$i]; - my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d}); - $colwidth[$i] = $w if $w > $colwidth[$i]; -} -for my $t (@subtests) { - my $firstr; + my %newdirabbrevs = %dirabbrevs; + while (!have_duplicate(values %newdirabbrevs)) { + %dirabbrevs = %newdirabbrevs; + last if !have_slash(values %dirabbrevs); + %newdirabbrevs = %dirabbrevs; + for (values %newdirabbrevs) { + s{^[^/]*/}{}; + } + } + + my %times; + my @colwidth = ((0)x@dirs); for my $i (0..$#dirs) { my $d = $dirs[$i]; - $times{$prefixes{$d}.$t} = [get_times("$resultsdir/$prefixes{$d}$t.times")]; - my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}}; - my $w = length format_times($r,$u,$s,$firstr); + my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d}); $colwidth[$i] = $w if $w > $colwidth[$i]; - $firstr = $r unless defined $firstr; } -} -my $totalwidth = 3*@dirs+$descrlen; -$totalwidth += $_ for (@colwidth); - -binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; + for my $t (@subtests) { + my $firstr; + for my $i (0..$#dirs) { + my $d = $dirs[$i]; + $times{$prefixes{$d}.$t} = [get_times("$resultsdir/$prefixes{$d}$t.times")]; + my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}}; + my $w = length format_times($r,$u,$s,$firstr); + $colwidth[$i] = $w if $w > $colwidth[$i]; + $firstr = $r unless defined $firstr; + } + } + my $totalwidth = 3*@dirs+$descrlen; + $totalwidth += $_ for (@colwidth); -printf "%-${descrlen}s", "Test"; -for my $i (0..$#dirs) { - my $d = $dirs[$i]; - printf " %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d}); -} -print "\n"; -print "-"x$totalwidth, "\n"; -for my $t (@subtests) { - printf "%-${descrlen}s", $descrs{$t}; - my $firstr; + printf "%-${descrlen}s", "Test"; for my $i (0..$#dirs) { my $d = $dirs[$i]; - my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}}; - printf " %-$colwidth[$i]s", format_times($r,$u,$s,$firstr); - $firstr = $r unless defined $firstr; + printf " %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d}); } print "\n"; + print "-"x$totalwidth, "\n"; + for my $t (@subtests) { + printf "%-${descrlen}s", $descrs{$t}; + my $firstr; + for my $i (0..$#dirs) { + my $d = $dirs[$i]; + my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}}; + printf " %-$colwidth[$i]s", format_times($r,$u,$s,$firstr); + $firstr = $r unless defined $firstr; + } + print "\n"; + } } + +binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; + +print_default_results(); -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 3/7] perf/aggregate: implement codespeed JSON output Message-ID: <20180105091226.16083-4-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-4-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` Codespeed (https://github.com/tobami/codespeed/) is an open source project that can be used to track how some software performs over time. It stores performance test results in a database and can show nice graphs and charts on a web interface. As it can be interesting to use Codespeed to see how Git performance evolves over time and releases, let's implement a Codespeed output in "perf/aggregate.perl". Helped-by: Eric Sunshine Signed-off-by: Christian Couder --- t/perf/aggregate.perl | 64 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 3609cb5dc3..5c439f6bc2 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -3,6 +3,7 @@ use lib '../../perl/blib/lib'; use strict; use warnings; +use JSON; use Git; sub get_times { @@ -35,10 +36,15 @@ sub format_times { return $out; } -my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests); +my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed); while (scalar @ARGV) { my $arg = $ARGV[0]; my $dir; + if ($arg eq "--codespeed") { + $codespeed = 1; + shift @ARGV; + next; + } last if -f $arg or $arg eq "--"; if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); @@ -70,8 +76,10 @@ if (not @tests) { } my $resultsdir = "test-results"; +my $results_section = ""; if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") { $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION}; + $results_section = $ENV{GIT_PERF_SUBSECTION}; } my @subtests; @@ -174,6 +182,58 @@ sub print_default_results { } } +sub print_codespeed_results { + my ($results_section) = @_; + + my $project = "Git"; + + my $executable = `uname -s -m`; + chomp $executable; + + if ($results_section ne "") { + $executable .= ", " . $results_section; + } + + my $environment; + if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { + $environment = $ENV{GIT_PERF_REPO_NAME}; + } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { + $environment = $ENV{GIT_TEST_INSTALLED}; + $environment =~ s|/bin-wrappers$||; + } else { + $environment = `uname -r`; + chomp $environment; + } + + my @data; + + for my $t (@subtests) { + for my $d (@dirs) { + my $commitid = $prefixes{$d}; + $commitid =~ s/^build_//; + $commitid =~ s/\.$//; + my ($result_value, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.times"); + + my %vals = ( + "commitid" => $commitid, + "project" => $project, + "branch" => $dirnames{$d}, + "executable" => $executable, + "benchmark" => $shorttests{$t} . " " . read_descr("$resultsdir/$t.descr"), + "environment" => $environment, + "result_value" => $result_value, + ); + push @data, \%vals; + } + } + + print to_json(\@data, {utf8 => 1, pretty => 1}), "\n"; +} + binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; -print_default_results(); +if ($codespeed) { + print_codespeed_results($results_section); +} else { + print_default_results(); +} -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 5/7] perf/run: learn about perf.codespeedOutput Message-ID: <20180105091226.16083-6-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-6-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` Let's make it possible to set in a config file the output format (regular or codespeed) of the perf tests. Signed-off-by: Christian Couder --- t/perf/run | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/t/perf/run b/t/perf/run index 214d658172..4e62d6bb3f 100755 --- a/t/perf/run +++ b/t/perf/run @@ -144,10 +144,15 @@ run_subsection () { set -- . "$@" fi + codespeed_opt= + test "$GIT_PERF_CODESPEED_OUTPUT" = "true" && codespeed_opt="--codespeed" + run_dirs "$@" - ./aggregate.perl "$@" + ./aggregate.perl $codespeed_opt "$@" } +get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf" "codespeedOutput" "--bool" + cd "$(dirname $0)" . ../../GIT-BUILD-OPTIONS -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 4/7] perf/run: add conf_opts argument to get_var_from_env_or_config() Message-ID: <20180105091226.16083-5-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-5-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` Let's make it possible to use `git config` type specifiers like `--int` or `--bool`, so that config values are converted to the canonical form and easier to use. This additional argument is now the fourth argument of get_var_from_env_or_config() instead of the fifth because we want the default value argument to be unset if it is not passed, and this is simpler if it is the last argument. Signed-off-by: Christian Couder --- t/perf/run | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/t/perf/run b/t/perf/run index 43e4de49ef..214d658172 100755 --- a/t/perf/run +++ b/t/perf/run @@ -105,7 +105,8 @@ get_var_from_env_or_config () { env_var="$1" conf_sec="$2" conf_var="$3" - # $4 can be set to a default value + conf_opts="$4" # optional + # $5 can be set to a default value # Do nothing if the env variable is already set eval "test -z \"\${$env_var+x}\"" || return @@ -116,18 +117,18 @@ get_var_from_env_or_config () { if test -n "$GIT_PERF_SUBSECTION" then var="$conf_sec.$GIT_PERF_SUBSECTION.$conf_var" - conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") && + conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" "$var") && eval "$env_var=\"$conf_value\"" && return fi var="$conf_sec.$conf_var" - conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") && + conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" "$var") && eval "$env_var=\"$conf_value\"" && return - test -n "${4+x}" && eval "$env_var=\"$4\"" + test -n "${5+x}" && eval "$env_var=\"$5\"" } run_subsection () { - get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" 3 + get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" "--int" 3 export GIT_PERF_REPEAT_COUNT get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf" "dirsOrRevs" -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 6/7] perf/run: learn to send output to codespeed server Message-ID: <20180105091226.16083-7-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-7-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` Let's make it possible to set in a config file the URL of a codespeed server. And then let's make the `run` script send the perf test results to this URL at the end of the tests. This should make is possible to easily automate the process of running perf tests and having their results available in Codespeed. Signed-off-by: Christian Couder --- t/perf/run | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/t/perf/run b/t/perf/run index 4e62d6bb3f..ef56396546 100755 --- a/t/perf/run +++ b/t/perf/run @@ -148,10 +148,20 @@ run_subsection () { test "$GIT_PERF_CODESPEED_OUTPUT" = "true" && codespeed_opt="--codespeed" run_dirs "$@" - ./aggregate.perl $codespeed_opt "$@" + + if test -z "$GIT_PERF_SEND_TO_CODESPEED" + then + ./aggregate.perl $codespeed_opt "$@" + else + json_res_file="test-results/$GIT_PERF_SUBSECTION/aggregate.json" + ./aggregate.perl --codespeed "$@" | tee "$json_res_file" + send_data_url="$GIT_PERF_SEND_TO_CODESPEED/result/add/json/" + curl -v --request POST --data-urlencode "json=$(cat "$json_res_file")" "$send_data_url" + fi } get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf" "codespeedOutput" "--bool" +get_var_from_env_or_config "GIT_PERF_SEND_TO_CODESPEED" "perf" "sendToCodespeed" cd "$(dirname $0)" . ../../GIT-BUILD-OPTIONS -- 2.16.0.rc0.40.gbe5e688583 ``` ## Christian Couder, 2018-01-05 09:12 Subject: [PATCH v3 7/7] perf/run: read GIT_PERF_REPO_NAME from perf.repoName Message-ID: <20180105091226.16083-8-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180105091226.16083-8-chriscool%40tuxfamily.org In-Reply-To: <20180105091226.16083-1-chriscool@tuxfamily.org> ``` The GIT_PERF_REPO_NAME env variable is used in the `aggregate.perl` script to set the 'environment' field in the JSON Codespeed output. Let's make it easy to set this variable by setting it in a config file. Signed-off-by: Christian Couder --- t/perf/run | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/perf/run b/t/perf/run index ef56396546..1a100d6134 100755 --- a/t/perf/run +++ b/t/perf/run @@ -137,6 +137,9 @@ run_subsection () { get_var_from_env_or_config "GIT_PERF_MAKE_COMMAND" "perf" "makeCommand" get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf" "makeOpts" + get_var_from_env_or_config "GIT_PERF_REPO_NAME" "perf" "repoName" + export GIT_PERF_REPO_NAME + GIT_PERF_AGGREGATING_LATER=t export GIT_PERF_AGGREGATING_LATER -- 2.16.0.rc0.40.gbe5e688583 ```