Re: [PATCH 1/2] t/perf: time rev-list with UNINTERESTING commits
On Mon, Jan 20, 2014 at 11:32:12PM +0100, Thomas Rast wrote: > > This is bad to be touching the repo and assuming it is non-bare. For > > some reason I assumed that the perf suite made a copy of the repo, but > > it doesn't. If you point to a bare repo via GIT_PERF_REPO, this part of > > the test fails. > > It does make a copy, but with cp -Rl. I haven't actually ever tried > what happens if you point it at a bare though. It *should* fail because > it tries to cd $repo/.git, but if that was itself bare... Oh, hmph. I checked my linux repo, which I had used as GIT_PERF_REPO, and noticed that it had the test commit in its reflog. But I forgot that is because I did the test manually there right before writing up the t/perf script! So yes, it copies, and it's totally fine to be modifying the repo. Bare repos seem to work just fine for me. It looks like we use `git rev-parse --git-dir` to get the source, and then copy that to `.git` in the temporary directory. So that works fine either way, and we do have a directory available as the working dir. But of course the config from the bare repo says `core.bare = true`, so some commands will bail. We could perhaps just set GIT_WORK_TREE in the perf scripts, which I believe would override the bare setting in the .git/config. And then we know the repos will be consistently non-bare. Whether we do that or not, I think the update I posted is preferable, as it reproduces the problem in a much simpler manner. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t/perf: time rev-list with UNINTERESTING commits
Jeff King writes: > On Mon, Jan 20, 2014 at 04:31:01PM -0500, Jeff King wrote: > >> diff --git a/t/perf/p0001-rev-list.sh b/t/perf/p0001-rev-list.sh >> index 4f71a63..b7258a7 100755 >> --- a/t/perf/p0001-rev-list.sh >> +++ b/t/perf/p0001-rev-list.sh >> @@ -14,4 +14,21 @@ test_perf 'rev-list --all --objects' ' >> git rev-list --all --objects >/dev/null >> ' >> >> +test_expect_success 'create new unreferenced commit' ' >> +git checkout --detach HEAD && >> +echo content >>file && >> +git add file && >> +git commit -m detached && >> +commit=$(git rev-parse --verify HEAD) && >> +git checkout - >> +' > > This is bad to be touching the repo and assuming it is non-bare. For > some reason I assumed that the perf suite made a copy of the repo, but > it doesn't. If you point to a bare repo via GIT_PERF_REPO, this part of > the test fails. It does make a copy, but with cp -Rl. I haven't actually ever tried what happens if you point it at a bare though. It *should* fail because it tries to cd $repo/.git, but if that was itself bare... -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t/perf: time rev-list with UNINTERESTING commits
On Mon, Jan 20, 2014 at 04:31:01PM -0500, Jeff King wrote:
> diff --git a/t/perf/p0001-rev-list.sh b/t/perf/p0001-rev-list.sh
> index 4f71a63..b7258a7 100755
> --- a/t/perf/p0001-rev-list.sh
> +++ b/t/perf/p0001-rev-list.sh
> @@ -14,4 +14,21 @@ test_perf 'rev-list --all --objects' '
> git rev-list --all --objects >/dev/null
> '
>
> +test_expect_success 'create new unreferenced commit' '
> + git checkout --detach HEAD &&
> + echo content >>file &&
> + git add file &&
> + git commit -m detached &&
> + commit=$(git rev-parse --verify HEAD) &&
> + git checkout -
> +'
This is bad to be touching the repo and assuming it is non-bare. For
some reason I assumed that the perf suite made a copy of the repo, but
it doesn't. If you point to a bare repo via GIT_PERF_REPO, this part of
the test fails.
It's actually enough to demonstrate the problem without changing the
tree at all. So this produces the same numbers, and works everywhere:
diff --git a/t/perf/p0001-rev-list.sh b/t/perf/p0001-rev-list.sh
index b7258a7..16359d5 100755
--- a/t/perf/p0001-rev-list.sh
+++ b/t/perf/p0001-rev-list.sh
@@ -15,12 +15,7 @@ test_perf 'rev-list --all --objects' '
'
test_expect_success 'create new unreferenced commit' '
- git checkout --detach HEAD &&
- echo content >>file &&
- git add file &&
- git commit -m detached &&
- commit=$(git rev-parse --verify HEAD) &&
- git checkout -
+ commit=$(git commit-tree HEAD^{tree} -p HEAD)
'
test_perf 'rev-list $commit --not --all' '
It still modifies the test repo, but at least in a fairly innocuous way.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t/perf: time rev-list with UNINTERESTING commits
We time a straight "rev-list --all" and its "--object" counterpart, both going all the way to the root. However, we do not time a partial history walk. This patch adds an extreme case: a walk over a very small slice of history, but with a very large set of UNINTERESTING tips. This is similar to the connectivity check run by git on a small fetch, or the walk done by any pre-receive hooks that want to check incoming commits. This test reveals a performance regression in git v1.8.4.2, caused by fbd4a70 (list-objects: mark more commits as edges in mark_edges_uninteresting, 2013-08-16): Test fbd4a703^ fbd4a703 -- 0001.1: rev-list --all 0.69(0.67+0.02) 0.69(0.68+0.01) +0.0% 0001.2: rev-list --all --objects 3.47(3.44+0.02) 3.48(3.44+0.03) +0.3% 0001.4: rev-list $commit --not --all 0.04(0.04+0.00) 0.04(0.04+0.00) +0.0% 0001.5: rev-list --objects $commit --not --all 0.04(0.03+0.00) 0.27(0.24+0.02) +575.0% Signed-off-by: Jeff King --- Sorry for the long lines in the commit message. I'm open to suggestions on reformatting. t/perf/p0001-rev-list.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/perf/p0001-rev-list.sh b/t/perf/p0001-rev-list.sh index 4f71a63..b7258a7 100755 --- a/t/perf/p0001-rev-list.sh +++ b/t/perf/p0001-rev-list.sh @@ -14,4 +14,21 @@ test_perf 'rev-list --all --objects' ' git rev-list --all --objects >/dev/null ' +test_expect_success 'create new unreferenced commit' ' + git checkout --detach HEAD && + echo content >>file && + git add file && + git commit -m detached && + commit=$(git rev-parse --verify HEAD) && + git checkout - +' + +test_perf 'rev-list $commit --not --all' ' + git rev-list $commit --not --all >/dev/null +' + +test_perf 'rev-list --objects $commit --not --all' ' + git rev-list --objects $commit --not --all >/dev/null +' + test_done -- 1.8.5.2.500.g8060133 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
