threads / patch / 5719

patchrunstatus: do not recurse into subdirectories if not needed

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

## tl;dr

5 messages between Sep 27, 2006 and Sep 28, 2006. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin· Sep 27, 2006, 11:16 UTC · lore

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(-)
Show changes to dir.c +15 −9
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
Junio C Hamano· Sep 28, 2006, 00:09 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 28 quoted lines
> 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).

Johannes Schindelin· Sep 28, 2006, 00:26 UTC · re: Junio C Hamano · lore

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

Hi,
On Wed, 27 Sep 2006, Junio C Hamano wrote:
Show 37 quoted lines
> 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

Johannes Schindelin· Sep 28, 2006, 00:44 UTC · re: Johannes Schindelin · lore

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

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(-)
Show changes to dir.c +15 −12
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;
 }
Junio C Hamano· Sep 28, 2006, 04:40 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 12 quoted lines
> 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.

← back to recent threads