[PATCH] runstatus: do not recurse into subdirectories if not needed

[PATCH] runstatus: do not recurse into subdirectories if not needed

[Johannes Schindelin]

This speeds up the case when you run git-status, having an untracked
subdirectory containing huge amounts of files.

It also clarifies the handling of hide_empty_directories; the old version
worked, but was hard to understand.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 dir.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/dir.c b/dir.c
index e2f472b..e69663c 100644
--- a/dir.c
+++ b/dir.c
@@ -274,6 +274,15 @@ static int dir_exists(const char *dirnam
 	return !strncmp(active_cache[pos]->name, dirname, len);
 }
 
+static int dir_is_empty(const char *dirname)
+{
+	DIR *fdir = opendir(dirname);
+	int result = (readdir(fdir) == NULL);
+
+	closedir(fdir);
+	return result;
+}
+
 /*
  * Read a directory tree. We currently ignore anything but
  * directories, regular files and symlinks. That's because git
@@ -314,7 +323,6 @@ static int read_directory_recursive(stru
 
 			switch (DTYPE(de)) {
 			struct stat st;
-			int subdir, rewind_base;
 			default:
 				continue;
 			case DT_UNKNOWN:
@@ -328,18 +336,16 @@ static int read_directory_recursive(stru
 			case DT_DIR:
 				memcpy(fullname + baselen + len, "/", 2);
 				len++;
-				rewind_base = dir->nr;
-				subdir = read_directory_recursive(dir, fullname, fullname,
-				                        baselen + len);
 				if (dir->show_other_directories &&
-				    (subdir || !dir->hide_empty_directories) &&
 				    !dir_exists(fullname, baselen + len)) {
-					/* Rewind the read subdirectory */
-					while (dir->nr > rewind_base)
-						free(dir->entries[--dir->nr]);
+					if (dir->hide_empty_directories &&
+					    dir_is_empty(fullname))
+						continue;
 					break;
 				}
-				contents += subdir;
+
+				contents += read_directory_recursive(dir,
+					fullname, fullname, baselen + len);
 				continue;
 			case DT_REG:
 			case DT_LNK:
-- 
1.4.2.1.g78cd-dirty

Re: [PATCH] runstatus: do not recurse into subdirectories if not needed

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This speeds up the case when you run git-status, having an untracked
> subdirectory containing huge amounts of files.
>
> It also clarifies the handling of hide_empty_directories; the old version
> worked, but was hard to understand.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
>  dir.c |   24 +++++++++++++++---------
>  1 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index e2f472b..e69663c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -274,6 +274,15 @@ static int dir_exists(const char *dirnam
>  	return !strncmp(active_cache[pos]->name, dirname, len);
>  }
>  
> +static int dir_is_empty(const char *dirname)
> +{
> +	DIR *fdir = opendir(dirname);
> +	int result = (readdir(fdir) == NULL);
> +
> +	closedir(fdir);
> +	return result;
> +}
> +

Does this really check if the directory is empty (I think you
would read "." and ".." out of it at least)?

When the original code recurses into subdirectory, it seems to
behave identically for a truly empty directory and a directory
that has only ".git" (or excluded files in it under !show_ignored).

Re: [PATCH] runstatus: do not recurse into subdirectories if not needed

[Johannes Schindelin]

Hi,

On Wed, 27 Sep 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This speeds up the case when you run git-status, having an untracked
> > subdirectory containing huge amounts of files.
> >
> > It also clarifies the handling of hide_empty_directories; the old version
> > worked, but was hard to understand.
> >
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > ---
> >  dir.c |   24 +++++++++++++++---------
> >  1 files changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/dir.c b/dir.c
> > index e2f472b..e69663c 100644
> > --- a/dir.c
> > +++ b/dir.c
> > @@ -274,6 +274,15 @@ static int dir_exists(const char *dirnam
> >  	return !strncmp(active_cache[pos]->name, dirname, len);
> >  }
> >  
> > +static int dir_is_empty(const char *dirname)
> > +{
> > +	DIR *fdir = opendir(dirname);
> > +	int result = (readdir(fdir) == NULL);
> > +
> > +	closedir(fdir);
> > +	return result;
> > +}
> > +
> 
> Does this really check if the directory is empty (I think you
> would read "." and ".." out of it at least)?
>
> When the original code recurses into subdirectory, it seems to
> behave identically for a truly empty directory and a directory
> that has only ".git" (or excluded files in it under !show_ignored).

Of course I missed that. Probably, because there is no test for that.

But given my experience of a very, _very_ slow git-status when I do not 
_care_ about that particular directory (even if it does contain .git and 
excluded files), is it really sensible to have _no_ way to disable 
hide_empty_directories when show_other_directories is enabled?

Or would it make sense to disable hide_empty_directories altogether?

Now, I could enhance dir_is_empty() to recursively test if the dir is 
empty, and return 0 on first sight of a not-excluded dir entry, but is it 
really worth the hassle?

Ciao,
Dscho

[PATCH 2nd try] runstatus: do not recurse into subdirectories if not needed

[Johannes Schindelin]

This speeds up the case when you run git-status, having an untracked
subdirectory containing huge amounts of files.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	On Thu, 28 Sep 2006, Johannes Schindelin wrote:

	> On Wed, 27 Sep 2006, Junio C Hamano wrote:
	> 
	> > Does this really check if the directory is empty (I think you
	> > would read "." and ".." out of it at least)?
	> >
	> > When the original code recurses into subdirectory, it seems to
	> > behave identically for a truly empty directory and a directory
	> > that has only ".git" (or excluded files in it under 
	> > !show_ignored).
	> 
	> Of course I missed that. Probably, because there is no test for 
	> that.
	>
	> [...] 
	> 
	> Now, I could enhance dir_is_empty() to recursively test if the 
	> dir is empty, and return 0 on first sight of a not-excluded dir 
	> entry, but is it really worth the hassle?

	Okay, so no more dir_is_empty(). Instead, read_directory_recursive()
	gets a flag. With this flag, "check_only", it exits as soon as it
	found valid entries, but does not add any. Way easier.

 dir.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/dir.c b/dir.c
index e2f472b..96389b3 100644
--- a/dir.c
+++ b/dir.c
@@ -283,7 +283,7 @@ static int dir_exists(const char *dirnam
  * Also, we ignore the name ".git" (even if it is not a directory).
  * That likely will not change.
  */
-static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen)
+static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int check_only)
 {
 	DIR *fdir = opendir(path);
 	int contents = 0;
@@ -314,7 +314,6 @@ static int read_directory_recursive(stru
 
 			switch (DTYPE(de)) {
 			struct stat st;
-			int subdir, rewind_base;
 			default:
 				continue;
 			case DT_UNKNOWN:
@@ -328,26 +327,30 @@ static int read_directory_recursive(stru
 			case DT_DIR:
 				memcpy(fullname + baselen + len, "/", 2);
 				len++;
-				rewind_base = dir->nr;
-				subdir = read_directory_recursive(dir, fullname, fullname,
-				                        baselen + len);
 				if (dir->show_other_directories &&
-				    (subdir || !dir->hide_empty_directories) &&
 				    !dir_exists(fullname, baselen + len)) {
-					/* Rewind the read subdirectory */
-					while (dir->nr > rewind_base)
-						free(dir->entries[--dir->nr]);
+					if (dir->hide_empty_directories &&
+					    !read_directory_recursive(dir,
+						    fullname, fullname,
+						    baselen + len, 1))
+						continue;
 					break;
 				}
-				contents += subdir;
+
+				contents += read_directory_recursive(dir,
+					fullname, fullname, baselen + len, 0);
 				continue;
 			case DT_REG:
 			case DT_LNK:
 				break;
 			}
-			add_name(dir, fullname, baselen + len);
 			contents++;
+			if (check_only)
+				goto exit_early;
+			else
+				add_name(dir, fullname, baselen + len);
 		}
+exit_early:
 		closedir(fdir);
 
 		pop_exclude_per_directory(dir, exclude_stk);
@@ -393,7 +396,7 @@ int read_directory(struct dir_struct *di
 		}
 	}
 
-	read_directory_recursive(dir, path, base, baselen);
+	read_directory_recursive(dir, path, base, baselen, 0);
 	qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
 	return dir->nr;
 }

Re: [PATCH 2nd try] runstatus: do not recurse into subdirectories if not needed

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This speeds up the case when you run git-status, having an untracked
> subdirectory containing huge amounts of files.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
> ---
>
> 	On Thu, 28 Sep 2006, Johannes Schindelin wrote:
>
> 	Okay, so no more dir_is_empty(). Instead, read_directory_recursive()
> 	gets a flag. With this flag, "check_only", it exits as soon as it
> 	found valid entries, but does not add any. Way easier.

Yeah, the logic is a lot easier to follow.