Skip to content

Commit 82dca95

Browse files
gitstergitster
committed
Merge branch 'ab/perf-installed-fix'
Performance test framework has been broken and measured the version of Git that happens to be on $PATH, not the specified one to measure, for a while, which has been corrected. * ab/perf-installed-fix: perf-lib.sh: forbid the use of GIT_TEST_INSTALLED perf tests: add "bindir" prefix to git tree test results perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh perf-lib.sh: make "./run <revisions>" use the correct gits perf aggregate: remove GIT_TEST_INSTALLED from --codespeed perf README: correct docs for 3c8f12c regression
2 parents f42bee7 + 82b7eb2 commit 82dca95

File tree

4 files changed

+53
-27
lines changed

4 files changed

+53
-27
lines changed

‎t/perf/README‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ call the aggregation script to summarize the results:
4545

4646
$ ./p0001-rev-list.sh
4747
[...]
48-
$ GIT_BUILD_DIR=/path/to/other/git ./p0001-rev-list.sh
48+
$ ./run /path/to/other/git -- ./p0001-rev-list.sh
4949
[...]
5050
$ ./aggregate.perl . /path/to/other/git ./p0001-rev-list.sh
5151

‎t/perf/aggregate.perl‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use warnings;
66
use Getopt::Long;
77
use Git;
8+
use Cwd qw(realpath);
89

910
sub get_times {
1011
my $name = shift;
@@ -98,18 +99,21 @@ sub format_size {
9899
while (scalar @ARGV) {
99100
my $arg = $ARGV[0];
100101
my $dir;
102+
my $prefix = '';
101103
last if -f $arg or $arg eq "--";
102104
if (! -d $arg) {
103105
my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
104106
$dir = "build/".$rev;
107+
} elsif ($arg eq '.') {
108+
$dir = '.';
105109
} else {
106-
$arg =~ s{/*$}{};
107-
$dir = $arg;
108-
$dirabbrevs{$dir} = $dir;
110+
$dir = realpath($arg);
111+
$dirnames{$dir} = $dir;
112+
$prefix .= 'bindir';
109113
}
110114
push @dirs, $dir;
111-
$dirnames{$dir} = $arg;
112-
my $prefix = $dir;
115+
$dirnames{$dir} ||= $arg;
116+
$prefix .= $dir;
113117
$prefix =~ tr/^a-zA-Z0-9/_/c;
114118
$prefixes{$dir} = $prefix . '.';
115119
shift @ARGV;
@@ -311,9 +315,6 @@ sub print_codespeed_results {
311315
$environment = $reponame;
312316
} elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
313317
$environment = $ENV{GIT_PERF_REPO_NAME};
314-
} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
315-
$environment = $ENV{GIT_TEST_INSTALLED};
316-
$environment =~ s|/bin-wrappers$||;
317318
} else {
318319
$environment = `uname -r`;
319320
chomp $environment;

‎t/perf/perf-lib.sh‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,21 @@
2121
# because it will change our working directory.
2222
TEST_DIRECTORY=$(pwd)/..
2323
TEST_OUTPUT_DIRECTORY=$(pwd)
24-
ABSOLUTE_GIT_TEST_INSTALLED=$(
25-
test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd)
2624

2725
TEST_NO_CREATE_REPO=t
2826
TEST_NO_MALLOC_CHECK=t
2927

3028
. ../test-lib.sh
3129

32-
if test -z "$GIT_TEST_INSTALLED"; then
33-
perf_results_prefix=
34-
else
35-
perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
36-
GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED
30+
if test -n "$GIT_TEST_INSTALLED" -a -z "$PERF_SET_GIT_TEST_INSTALLED"
31+
then
32+
error "Do not use GIT_TEST_INSTALLED with the perf tests.
33+
34+
Instead use:
35+
36+
./run <path-to-git> -- <tests>
37+
38+
See t/perf/README for details."
3739
fi
3840

3941
# Variables from test-lib that are normally internal to the tests; we
@@ -179,7 +181,7 @@ test_wrapper_ () {
179181
base=$(basename "$0" .sh)
180182
echo "$test_count" >>"$perf_results_dir"/$base.subtests
181183
echo "$1" >"$perf_results_dir"/$base.$test_count.descr
182-
base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
184+
base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"
183185
"$test_wrapper_func_" "$@"
184186
fi
185187

‎t/perf/run‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,24 @@ build_git_rev () {
7070
) || die "failed to build revision '$mydir'"
7171
}
7272

73+
set_git_test_installed () {
74+
mydir=$1
75+
76+
mydir_abs=$(cd $mydir && pwd)
77+
mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers"
78+
if test -d "$mydir_abs_wrappers"
79+
then
80+
GIT_TEST_INSTALLED=$mydir_abs_wrappers
81+
else
82+
# Older versions of git lacked bin-wrappers;
83+
# fallback to the files in the root.
84+
GIT_TEST_INSTALLED=$mydir_abs
85+
fi
86+
export GIT_TEST_INSTALLED
87+
PERF_SET_GIT_TEST_INSTALLED=true
88+
export PERF_SET_GIT_TEST_INSTALLED
89+
}
90+
7391
run_dirs_helper () {
7492
mydir=${1%/}
7593
shift
@@ -79,24 +97,29 @@ run_dirs_helper () {
7997
if test $# -gt 0 -a "$1" = --; then
8098
shift
8199
fi
82-
if [ ! -d "$mydir" ]; then
100+
101+
PERF_RESULTS_PREFIX=
102+
if test "$mydir" = "."
103+
then
104+
unset GIT_TEST_INSTALLED
105+
elif test -d "$mydir"
106+
then
107+
PERF_RESULTS_PREFIX=bindir$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_").
108+
set_git_test_installed "$mydir"
109+
else
83110
rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
84111
die "'$mydir' is neither a directory nor a valid revision"
85112
if [ ! -d build/$rev ]; then
86113
unpack_git_rev $rev
87114
fi
88115
build_git_rev $rev "$mydir"
89116
mydir=build/$rev
117+
118+
PERF_RESULTS_PREFIX=build_$rev.
119+
set_git_test_installed "$mydir"
90120
fi
91-
if test "$mydir" = .; then
92-
unset GIT_TEST_INSTALLED
93-
else
94-
GIT_TEST_INSTALLED="$mydir/bin-wrappers"
95-
# Older versions of git lacked bin-wrappers; fallback to the
96-
# files in the root.
97-
test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$mydir
98-
export GIT_TEST_INSTALLED
99-
fi
121+
export PERF_RESULTS_PREFIX
122+
100123
run_one_dir "$@"
101124
}
102125

0 commit comments

Comments
 (0)