| Message ID | f248b41d6e2df2d34a4304e2655df8cb094483e9.1621451532.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Speed up connectivity checks via quarantine dir | expand |
On Wed, May 19, 2021 at 1:22 PM Patrick Steinhardt <ps@pks.im> wrote: > We'll the connectivity check logic for git-receive-pack(1) in the s/the conn/do the conn/ > diff --git a/t/perf/p5400-receive-pack.sh b/t/perf/p5400-receive-pack.sh > new file mode 100755 > index 0000000000..2b0c89d977 > --- /dev/null > +++ b/t/perf/p5400-receive-pack.sh > @@ -0,0 +1,74 @@ > +#!/bin/sh This code is, unfortunately, full of bash-isms: > + refs=("create updated:new") Plain sh doesn't have arrays... > + done < <(printf "%s\n" "${refs[@]}") This is another bash-ism. > +done < <(printf "%s\n" "clone $TARGET_REPO_CLONE" "extrarefs $TARGET_REPO_REFS" "empty $TARGET_REPO_EMPTY") I think these are mostly easily corrected but the `refs` probably should just be dumped into a file, one line at a time, to be re-read from a file, since `printf ... | while read ...` runs the whole loop inside a subshell. Chris
On Wed, May 19, 2021 at 09:13:27PM +0200, Patrick Steinhardt wrote: > +while read name repo > +do > + refs=("create updated:new") This (and the other array manipulation below) is a bash-ism. Presumably you've got TEST_SHELL_PATH pointed at bash. Without that, on a system where /bin/sh is dash, the script chokes here. For your purposes here, I think you can get by with just a single string with newlines in it. Or even a file (see below). > + while read desc ref > + do > + test_expect_success "setup $name $desc" " > + test_must_fail git push --force '$repo' '$ref' \ > + --receive-pack='tee pack | git receive-pack' 2>err && > + grep 'failed in pre-receive hook' err > + " This inverts the double- and single- quotes from our usual style. So if $repo is "foo", you are creating a string that has: test_must_fail git push --force 'foo' ... in it, and then the test harness will eval that string. That will fail if $repo itself contains a single quote. Pretty unlikely, but I think it contains the absolute path. The usual style is: test_expect_success "setup $name $desc" ' test_must_fail git push --force "$repo" "$ref" \ ...etc... ' where the variables are dereferenced inside the eval'd snippet. So no quoting is necessary, no matter what's in the variables. > + test_perf "receive-pack $name $desc" " > + git receive-pack '$repo' <pack >negotiation && > + grep 'pre-receive hook declined' negotiation > + " Likewise here, but note that test_perf is tricky! It runs the snippet in a sub-process. You have to export $repo to make it visible (you can use test_export, but you don't need to; there's some discussing in t/perf/README). > + done < <(printf "%s\n" "${refs[@]}") > +done < <(printf "%s\n" "clone $TARGET_REPO_CLONE" "extrarefs $TARGET_REPO_REFS" "empty $TARGET_REPO_EMPTY") These process substitutions are also bash-isms. I guess you're trying to avoid putting the while on the right-hand side of a pipe like: printf "%s\n" ... | while read ... which is good, because they set variables and those values don't reliably make it out of the pipeline. If you stick the contents of $refs into a file, then you can just do: while read ... do ... done <ref-descs For the outer one, a here-doc is probably a bit simpler: while read ... do ... done <<-EOF clone $TARGET_REPO_CLONE extrarefs $TARGET_REPO_REFS empty $TARGET_REPO_EMPTY EOF -Peff
On Wed, May 19, 2021 at 09:13:27PM +0200, Patrick Steinhardt wrote: > We'll the connectivity check logic for git-receive-pack(1) in the > following commits to make it perform better. As a preparatory step, add > some benchmarks such that we can measure these changes. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > t/perf/p5400-receive-pack.sh | 74 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100755 t/perf/p5400-receive-pack.sh > > diff --git a/t/perf/p5400-receive-pack.sh b/t/perf/p5400-receive-pack.sh > new file mode 100755 > index 0000000000..2b0c89d977 > --- /dev/null > +++ b/t/perf/p5400-receive-pack.sh > @@ -0,0 +1,74 @@ > +#!/bin/sh > + > +test_description="Tests performance of receive-pack" > + > +. ./perf-lib.sh > + > +test_perf_large_repo > + > +test_expect_success 'setup' ' > + # Create a main branch such that we do not have to rely on any specific > + # branch to exist in the perf repository. > + git switch --force-create main && > + > + TARGET_REPO_CLONE=$(pwd)/target-clone.git && > + git clone --bare --dissociate --branch main "$(pwd)" "$TARGET_REPO_CLONE" && > + TARGET_REPO_REFS=$(pwd)/target-refs.git && > + git clone --bare --dissociate --branch main "$(pwd)" "$TARGET_REPO_REFS" && > + TARGET_REPO_EMPTY=$(pwd)/target-empty.git && > + git init --bare "$TARGET_REPO_EMPTY" && > + > + # Set up a pre-receive hook such that no refs will ever be changed. > + # This easily allows multiple perf runs, but still exercises > + # server-side reference negotiation and checking for consistency. > + mkdir hooks && > + write_script hooks/pre-receive <<-EOF && > + #!/bin/sh Nit: the 'write_script' helper writes a shebang line, so this is unnecessary. > + echo "failed in pre-receive hook" > + exit 1 > + EOF
diff --git a/t/perf/p5400-receive-pack.sh b/t/perf/p5400-receive-pack.sh new file mode 100755 index 0000000000..2b0c89d977 --- /dev/null +++ b/t/perf/p5400-receive-pack.sh @@ -0,0 +1,74 @@ +#!/bin/sh + +test_description="Tests performance of receive-pack" + +. ./perf-lib.sh + +test_perf_large_repo + +test_expect_success 'setup' ' + # Create a main branch such that we do not have to rely on any specific + # branch to exist in the perf repository. + git switch --force-create main && + + TARGET_REPO_CLONE=$(pwd)/target-clone.git && + git clone --bare --dissociate --branch main "$(pwd)" "$TARGET_REPO_CLONE" && + TARGET_REPO_REFS=$(pwd)/target-refs.git && + git clone --bare --dissociate --branch main "$(pwd)" "$TARGET_REPO_REFS" && + TARGET_REPO_EMPTY=$(pwd)/target-empty.git && + git init --bare "$TARGET_REPO_EMPTY" && + + # Set up a pre-receive hook such that no refs will ever be changed. + # This easily allows multiple perf runs, but still exercises + # server-side reference negotiation and checking for consistency. + mkdir hooks && + write_script hooks/pre-receive <<-EOF && + #!/bin/sh + echo "failed in pre-receive hook" + exit 1 + EOF + cat >config <<-EOF && + [core] + hooksPath=$(pwd)/hooks + EOF + GIT_CONFIG_GLOBAL="$(pwd)/config" && + export GIT_CONFIG_GLOBAL && + + # Create a reference for each commit in the target repository with + # extra-refs. While this may be an atypical setup, biggish repositories + # easily end up with hundreds of thousands of refs, and this is a good + # enough approximation. + git -C "$TARGET_REPO_REFS" log --all --format="tformat:create refs/commit/%h %H" | + git -C "$TARGET_REPO_REFS" update-ref --stdin && + + git switch --create updated && + test_commit --no-tag updated +' + +while read name repo +do + refs=("create updated:new") + + # If the target repository is the empty one, then the only thing we can + # do is to create a new branch. + if test "$repo" != "$TARGET_REPO_EMPTY" + then + refs+=("update updated:main" "reset main~:main" "delete :main") + fi + + while read desc ref + do + test_expect_success "setup $name $desc" " + test_must_fail git push --force '$repo' '$ref' \ + --receive-pack='tee pack | git receive-pack' 2>err && + grep 'failed in pre-receive hook' err + " + + test_perf "receive-pack $name $desc" " + git receive-pack '$repo' <pack >negotiation && + grep 'pre-receive hook declined' negotiation + " + done < <(printf "%s\n" "${refs[@]}") +done < <(printf "%s\n" "clone $TARGET_REPO_CLONE" "extrarefs $TARGET_REPO_REFS" "empty $TARGET_REPO_EMPTY") + +test_done
We'll the connectivity check logic for git-receive-pack(1) in the following commits to make it perform better. As a preparatory step, add some benchmarks such that we can measure these changes. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- t/perf/p5400-receive-pack.sh | 74 ++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100755 t/perf/p5400-receive-pack.sh