threads / discuss / 22698

'git status' on NFS performance regression in 1.7.0

Subject: 'git status' on NFS performance regression in 1.7.0

## tl;dr

6 messages between Feb 17, 2010 and Feb 18, 2010.

replies: 5people: 3as markdown or json

James Pickens· Feb 17, 2010, 20:08 UTC · lore
Hi,

I noticed that 'git status' in version 1.7.0 is much slower than in 1.6.2.5 on large work trees on NFS - averaging ~13 seconds runtime vs. ~2 seconds. I did a bit of debugging and found that 'git status' apparently doesn't use the multi-threaded preload_index any more, although some other commands like diff still use it. Was it intentionally dropped from 'git status'?

James
Junio C Hamano· Feb 17, 2010, 20:22 UTC · re: James Pickens · lore

Re: 'git status' on NFS performance regression in 1.7.0

James Pickens <jepicken@gmail.com> writes:
Show 5 quoted lines
> I noticed that 'git status' in version 1.7.0 is much slower than in 1.6.2.5
> on large work trees on NFS - averaging ~13 seconds runtime vs. ~2 seconds.
> I did a bit of debugging and found that 'git status' apparently doesn't use
> the multi-threaded preload_index any more, although some other commands
> like diff still use it.  Was it intentionally dropped from 'git status'?

There might be subtle breakage for doing this, but it would be worth a try ;-)

 builtin-commit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 55676fd..71f81c9 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1046,7 +1046,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	if (*argv)
 		s.pathspec = get_pathspec(prefix, argv);
 
-	read_cache();
+	read_cache_preload();
 	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);
 	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
 	s.in_merge = in_merge;
Junio C Hamano· Feb 17, 2010, 20:23 UTC · re: James Pickens · lore

Re: 'git status' on NFS performance regression in 1.7.0

James Pickens <jepicken@gmail.com> writes:
Show 5 quoted lines
> I noticed that 'git status' in version 1.7.0 is much slower than in 1.6.2.5
> on large work trees on NFS - averaging ~13 seconds runtime vs. ~2 seconds.
> I did a bit of debugging and found that 'git status' apparently doesn't use
> the multi-threaded preload_index any more, although some other commands
> like diff still use it.  Was it intentionally dropped from 'git status'?

There might be subtle breakage for doing this, but it would be worth a try ;-)

 builtin-commit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 55676fd..71f81c9 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1046,7 +1046,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	if (*argv)
 		s.pathspec = get_pathspec(prefix, argv);
 
-	read_cache();
+	read_cache_preload(NULL);
 	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);
 	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
 	s.in_merge = in_merge;
James Pickens· Feb 17, 2010, 21:35 UTC · re: Junio C Hamano · lore

Re: 'git status' on NFS performance regression in 1.7.0

On Wed, Feb 17, 2010, Junio C Hamano <gitster@pobox.com> wrote:
> There might be subtle breakage for doing this, but it would be worth a try
> ;-)

Thanks, with that patch Git 1.7.0 is faster than 1.6.2.5 - ~2 seconds average runtime vs. ~3 seconds (1.6.2.5 got slower since my original test last night; must be more network and/or file server traffic right now).

I'm not sure how to interpret the "subtle breakage" comment with the winking smiley. Do you mean that preload_index in 1.7.0 is not well tested and may be broken? FWIW, I didn't notice any breakage, but I didn't do much testing.

James
Junio C Hamano· Feb 17, 2010, 22:03 UTC · re: James Pickens · lore

Re: 'git status' on NFS performance regression in 1.7.0

James Pickens <jepicken@gmail.com> writes:
> I'm not sure how to interpret the "subtle breakage" comment with the
> winking smiley.  Do you mean that preload_index in 1.7.0 is not well tested
> and may be broken?  FWIW, I didn't notice any breakage, but I didn't do
> much testing.

The new "status" codepath is different from "commit --dry-run" codepath that was used by "git status" in 1.6.6 series. This old codepath has been used extensibly with preloaded index and is continued to be used when you run "git commit" with various options. It is not preload-index that could be subtly broken.

However, nobody used the new "status" codepath with preloaded index, and I haven't thought things through if anything we are doing in that codepath is incompatible with preloaded index in some way.

By the way, the argument to read_cache_preload() should be s.pathspec, not NULL, I think.

Thanks.
Peter Krefting· Feb 18, 2010, 08:46 UTC · re: James Pickens · lore

Re: 'git status' on NFS performance regression in 1.7.0

James Pickens:
> I noticed that 'git status' in version 1.7.0 is much slower than in 
> 1.6.2.5 on large work trees on NFS - averaging ~13 seconds runtime vs. ~2 
> seconds.

This applies to local disk as well. Where it used to be almost instantaneous, I now see a wait of about three seconds on a checkout of about 8,000 files.

-- 
\\// Peter - http://www.softwolves.pp.se/

← back to recent threads