| Message ID | bfaac619112b04aa6545f229ea60433cbf8da73a.1604501265.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | update-ref: Allow creation of multiple transactions | expand |
On Wed, Nov 04, 2020 at 03:57:22PM +0100, Patrick Steinhardt wrote: > In commit 0a0fbbe3ff (refs: remove lookup cache for > reference-transaction hook, 2020-08-25), a new benchmark was added to > p1400 which has the intention to exercise creation of multiple > transactions in a single process. As git-update-ref wasn't yet able to > create multiple transactions with a single run we instead used git-push. > As its non-atomic version creates a transaction per reference update, > this was the best approximation we could make at that point in time. > > Now that `git-update-ref --stdin` supports creation of multiple > transactions, let's convert the benchmark to use that instead. It has > less overhead and it's also a lot clearer what the actual intention is. Good direction. The diff confused me for a moment... > @@ -26,14 +27,7 @@ test_perf "update-ref" ' > ' > > test_perf "update-ref --stdin" ' > - git update-ref --stdin <update && > - git update-ref --stdin <delete && > - git update-ref --stdin <create > -' > - > -test_perf "nonatomic push" ' > - git push ./target-repo.git $(test_seq 1000) && > - git push --delete ./target-repo.git $(test_seq 1000) > + git update-ref --stdin <instructions >/dev/null > ' ...because we're dropping _two_ tests here. But I think they were testing the same thing, just with varying degrees of quality. It could possibly be useful to have perf results broken down by operation type (create vs delete vs update), but the original certainly didn't do that. And it's not clear to me it would actually produce interesting results; certainly not related to the hook, but possibly related to benchmarking ref updates in general. So I don't think it's worth worrying about. -Peff
diff --git a/t/perf/p1400-update-ref.sh b/t/perf/p1400-update-ref.sh index ce5ac3ed85..dda8a74866 100755 --- a/t/perf/p1400-update-ref.sh +++ b/t/perf/p1400-update-ref.sh @@ -7,13 +7,14 @@ test_description="Tests performance of update-ref" test_perf_fresh_repo test_expect_success "setup" ' - git init --bare target-repo.git && test_commit PRE && test_commit POST && - printf "create refs/heads/%d PRE\n" $(test_seq 1000) >create && - printf "update refs/heads/%d POST PRE\n" $(test_seq 1000) >update && - printf "delete refs/heads/%d POST\n" $(test_seq 1000) >delete && - git update-ref --stdin <create + for i in $(test_seq 5000) + do + printf "start\ncreate refs/heads/%d PRE\ncommit\n" $i && + printf "start\nupdate refs/heads/%d POST PRE\ncommit\n" $i && + printf "start\ndelete refs/heads/%d POST\ncommit\n" $i + done >instructions ' test_perf "update-ref" ' @@ -26,14 +27,7 @@ test_perf "update-ref" ' ' test_perf "update-ref --stdin" ' - git update-ref --stdin <update && - git update-ref --stdin <delete && - git update-ref --stdin <create -' - -test_perf "nonatomic push" ' - git push ./target-repo.git $(test_seq 1000) && - git push --delete ./target-repo.git $(test_seq 1000) + git update-ref --stdin <instructions >/dev/null ' test_done
In commit 0a0fbbe3ff (refs: remove lookup cache for reference-transaction hook, 2020-08-25), a new benchmark was added to p1400 which has the intention to exercise creation of multiple transactions in a single process. As git-update-ref wasn't yet able to create multiple transactions with a single run we instead used git-push. As its non-atomic version creates a transaction per reference update, this was the best approximation we could make at that point in time. Now that `git-update-ref --stdin` supports creation of multiple transactions, let's convert the benchmark to use that instead. It has less overhead and it's also a lot clearer what the actual intention is. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- t/perf/p1400-update-ref.sh | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)