On Mon, Jun 28 2021, Patrick Steinhardt wrote:
> [[PGP Signed Part:Undecided]]
> 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@xxxxxx>
> ---
> t/perf/p5400-receive-pack.sh | 97 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 97 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..a945e014a3
> --- /dev/null
> +++ b/t/perf/p5400-receive-pack.sh
> @@ -0,0 +1,97 @@
> +#!/bin/sh
> +
> +test_description="Tests performance of receive-pack"
> +
> +. ./perf-lib.sh
> +
> +test_perf_large_repo
>From the runtime I think this just needs test_perf_default_repo, no?
I.e. we should only have *_large_* for cases where git.git is too small
to produce meaningful results.
Part of th problem is that git.git has become larger over time...
> +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 &&
> +
> + # 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
You don't need the #!/bin/sh here, and it won't be used. write_script()
adds it (or the wanted shell path).
> + echo "failed in pre-receive hook"
> + exit 1
> + EOF
> + cat >config <<-EOF &&
> + [core]
> + hooksPath=$(pwd)/hooks
> + EOF
Easier understood IMO as:
git config -f config core.hooksPath ...
> + GIT_CONFIG_GLOBAL="$(pwd)/config" &&
> + export GIT_CONFIG_GLOBAL &&
> +
> + git switch --create updated &&
> + test_commit --no-tag updated
> +'
> +
> +setup_empty() {
> + git init --bare "$2"
> +}
I searched ahead for setup_empty, looked unused, but...
> +setup_clone() {
> + git clone --bare --no-local --branch main "$1" "$2"
> +}
> +
> +setup_clone_bitmap() {
> + git clone --bare --no-local --branch main "$1" "$2" &&
> + git -C "$2" repack -Adb
> +}
> +
> +# 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.
> +setup_extrarefs() {
> + git clone --bare --no-local --branch main "$1" "$2" &&
> + git -C "$2" log --all --format="tformat:create refs/commit/%h %H" |
> + git -C "$2" update-ref --stdin
> +}
> +
> +# 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.
> +setup_extrarefs_bitmap() {
> + git clone --bare --no-local --branch main "$1" "$2" &&
> + git -C "$2" log --all --format="tformat:create refs/commit/%h %H" |
> + git -C "$2" update-ref --stdin &&
> + git -C "$2" repack -Adb
> +}
> +
> +for repo in empty clone clone_bitmap extrarefs extrarefs_bitmap
> +do
> + test_expect_success "$repo setup" '
> + rm -rf target.git &&
> + setup_$repo "$(pwd)" target.git
...here we use it via interpolation.
I'd find this whole pattern much easier to understand if the setups were
just a bunch of test_expect_success that created a repo_empty.git,
repo_extrarefs.git etc. Then this loop would be:
for repo in repo*.git ...
I'd think that would also give you more meaningful perf data, as now the
OS will churn between the clone & the subsequent push tests, better to
do all the setup, then all the different perf tests.
Perhaps there's also a way to re-use this setup across different runs, I
don't know/can't remember if t/perf has a less transient thing than the
normal trash directory to use for that.
