{"thread":{"id":"5719","subject":"[PATCH] runstatus: do not recurse into subdirectories if not needed","startedAt":"2006-09-27T11:16:45Z","lastAt":"2006-09-28T04:40:50Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"27782","messageId":"Pine.LNX.4.63.0609271315470.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5719","inReplyTo":null,"subject":"[PATCH] runstatus: do not recurse into subdirectories if not needed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-09-27T11:16:45Z","receivedAt":"2006-09-27T11:16:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis speeds up the case when you run git-status, having an untracked\nsubdirectory containing huge amounts of files.\n\nIt also clarifies the handling of hide_empty_directories; the old version\nworked, but was hard to understand.\n\nSigned-off-by: Johannes Schindelin \n---\n dir.c | 24 +++++++++++++++---------\n 1 files changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex e2f472b..e69663c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -274,6 +274,15 @@ static int dir_exists(const char *dirnam\n \treturn !strncmp(active_cache[pos]->name, dirname, len);\n }\n \n+static int dir_is_empty(const char *dirname)\n+{\n+\tDIR *fdir = opendir(dirname);\n+\tint result = (readdir(fdir) == NULL);\n+\n+\tclosedir(fdir);\n+\treturn result;\n+}\n+\n /*\n * Read a directory tree. We currently ignore anything but\n * directories, regular files and symlinks. That's because git\n@@ -314,7 +323,6 @@ static int read_directory_recursive(stru\n \n \t\t\tswitch (DTYPE(de)) {\n \t\t\tstruct stat st;\n-\t\t\tint subdir, rewind_base;\n \t\t\tdefault:\n \t\t\t\tcontinue;\n \t\t\tcase DT_UNKNOWN:\n@@ -328,18 +336,16 @@ static int read_directory_recursive(stru\n \t\t\tcase DT_DIR:\n \t\t\t\tmemcpy(fullname + baselen + len, \"/\", 2);\n \t\t\t\tlen++;\n-\t\t\t\trewind_base = dir->nr;\n-\t\t\t\tsubdir = read_directory_recursive(dir, fullname, fullname,\n-\t\t\t\t baselen + len);\n \t\t\t\tif (dir->show_other_directories &&\n-\t\t\t\t (subdir || !dir->hide_empty_directories) &&\n \t\t\t\t !dir_exists(fullname, baselen + len)) {\n-\t\t\t\t\t/* Rewind the read subdirectory */\n-\t\t\t\t\twhile (dir->nr > rewind_base)\n-\t\t\t\t\t\tfree(dir->entries[--dir->nr]);\n+\t\t\t\t\tif (dir->hide_empty_directories &&\n+\t\t\t\t\t dir_is_empty(fullname))\n+\t\t\t\t\t\tcontinue;\n \t\t\t\t\tbreak;\n \t\t\t\t}\n-\t\t\t\tcontents += subdir;\n+\n+\t\t\t\tcontents += read_directory_recursive(dir,\n+\t\t\t\t\tfullname, fullname, baselen + len);\n \t\t\t\tcontinue;\n \t\t\tcase DT_REG:\n \t\t\tcase DT_LNK:\n-- \n1.4.2.1.g78cd-dirty\n"},{"id":"27833","messageId":"7vac4k97if.fsf@assigned-by-dhcp.cox.net","threadId":"5719","inReplyTo":"Pine.LNX.4.63.0609271315470.14200@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] runstatus: do not recurse into subdirectories if not needed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-28T00:09:28Z","receivedAt":"2006-09-28T00:09:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This speeds up the case when you run git-status, having an untracked\n> subdirectory containing huge amounts of files.\n>\n> It also clarifies the handling of hide_empty_directories; the old version\n> worked, but was hard to understand.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> dir.c | 24 +++++++++++++++---------\n> 1 files changed, 15 insertions(+), 9 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index e2f472b..e69663c 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -274,6 +274,15 @@ static int dir_exists(const char *dirnam\n> \treturn !strncmp(active_cache[pos]->name, dirname, len);\n> }\n> \n> +static int dir_is_empty(const char *dirname)\n> +{\n> +\tDIR *fdir = opendir(dirname);\n> +\tint result = (readdir(fdir) == NULL);\n> +\n> +\tclosedir(fdir);\n> +\treturn result;\n> +}\n> +\n\nDoes this really check if the directory is empty (I think you\nwould read \".\" and \"..\" out of it at least)?\n\nWhen the original code recurses into subdirectory, it seems to\nbehave identically for a truly empty directory and a directory\nthat has only \".git\" (or excluded files in it under !show_ignored).\n"},{"id":"27841","messageId":"Pine.LNX.4.63.0609280216350.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5719","inReplyTo":"7vac4k97if.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] runstatus: do not recurse into subdirectories if not needed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-09-28T00:26:51Z","receivedAt":"2006-09-28T00:26:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 27 Sep 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > This speeds up the case when you run git-status, having an untracked\n> > subdirectory containing huge amounts of files.\n> >\n> > It also clarifies the handling of hide_empty_directories; the old version\n> > worked, but was hard to understand.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > dir.c | 24 +++++++++++++++---------\n> > 1 files changed, 15 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/dir.c b/dir.c\n> > index e2f472b..e69663c 100644\n> > --- a/dir.c\n> > +++ b/dir.c\n> > @@ -274,6 +274,15 @@ static int dir_exists(const char *dirnam\n> > \treturn !strncmp(active_cache[pos]->name, dirname, len);\n> > }\n> > \n> > +static int dir_is_empty(const char *dirname)\n> > +{\n> > +\tDIR *fdir = opendir(dirname);\n> > +\tint result = (readdir(fdir) == NULL);\n> > +\n> > +\tclosedir(fdir);\n> > +\treturn result;\n> > +}\n> > +\n> \n> Does this really check if the directory is empty (I think you\n> would read \".\" and \"..\" out of it at least)?\n>\n> When the original code recurses into subdirectory, it seems to\n> behave identically for a truly empty directory and a directory\n> that has only \".git\" (or excluded files in it under !show_ignored).\n\nOf course I missed that. Probably, because there is no test for that.\n\nBut given my experience of a very, _very_ slow git-status when I do not \n_care_ about that particular directory (even if it does contain .git and \nexcluded files), is it really sensible to have _no_ way to disable \nhide_empty_directories when show_other_directories is enabled?\n\nOr would it make sense to disable hide_empty_directories altogether?\n\nNow, I could enhance dir_is_empty() to recursively test if the dir is \nempty, and return 0 on first sight of a not-excluded dir entry, but is it \nreally worth the hassle?\n\nCiao,\nDscho\n"},{"id":"27843","messageId":"Pine.LNX.4.63.0609280240060.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5719","inReplyTo":"Pine.LNX.4.63.0609280216350.14200@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH 2nd try] runstatus: do not recurse into subdirectories if not needed","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-09-28T00:44:30Z","receivedAt":"2006-09-28T00:44:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis speeds up the case when you run git-status, having an untracked\nsubdirectory containing huge amounts of files.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\tOn Thu, 28 Sep 2006, Johannes Schindelin wrote:\n\n\t> On Wed, 27 Sep 2006, Junio C Hamano wrote:\n\t> \n\t> > Does this really check if the directory is empty (I think you\n\t> > would read \".\" and \"..\" out of it at least)?\n\t> >\n\t> > When the original code recurses into subdirectory, it seems to\n\t> > behave identically for a truly empty directory and a directory\n\t> > that has only \".git\" (or excluded files in it under \n\t> > !show_ignored).\n\t> \n\t> Of course I missed that. Probably, because there is no test for \n\t> that.\n\t>\n\t> [...] \n\t> \n\t> Now, I could enhance dir_is_empty() to recursively test if the \n\t> dir is empty, and return 0 on first sight of a not-excluded dir \n\t> entry, but is it really worth the hassle?\n\n\tOkay, so no more dir_is_empty(). Instead, read_directory_recursive()\n\tgets a flag. With this flag, \"check_only\", it exits as soon as it\n\tfound valid entries, but does not add any. Way easier.\n\n dir.c | 27 +++++++++++++++------------\n 1 files changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex e2f472b..96389b3 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -283,7 +283,7 @@ static int dir_exists(const char *dirnam\n * Also, we ignore the name \".git\" (even if it is not a directory).\n * That likely will not change.\n */\n-static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen)\n+static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int check_only)\n {\n \tDIR *fdir = opendir(path);\n \tint contents = 0;\n@@ -314,7 +314,6 @@ static int read_directory_recursive(stru\n \n \t\t\tswitch (DTYPE(de)) {\n \t\t\tstruct stat st;\n-\t\t\tint subdir, rewind_base;\n \t\t\tdefault:\n \t\t\t\tcontinue;\n \t\t\tcase DT_UNKNOWN:\n@@ -328,26 +327,30 @@ static int read_directory_recursive(stru\n \t\t\tcase DT_DIR:\n \t\t\t\tmemcpy(fullname + baselen + len, \"/\", 2);\n \t\t\t\tlen++;\n-\t\t\t\trewind_base = dir->nr;\n-\t\t\t\tsubdir = read_directory_recursive(dir, fullname, fullname,\n-\t\t\t\t baselen + len);\n \t\t\t\tif (dir->show_other_directories &&\n-\t\t\t\t (subdir || !dir->hide_empty_directories) &&\n \t\t\t\t !dir_exists(fullname, baselen + len)) {\n-\t\t\t\t\t/* Rewind the read subdirectory */\n-\t\t\t\t\twhile (dir->nr > rewind_base)\n-\t\t\t\t\t\tfree(dir->entries[--dir->nr]);\n+\t\t\t\t\tif (dir->hide_empty_directories &&\n+\t\t\t\t\t !read_directory_recursive(dir,\n+\t\t\t\t\t\t fullname, fullname,\n+\t\t\t\t\t\t baselen + len, 1))\n+\t\t\t\t\t\tcontinue;\n \t\t\t\t\tbreak;\n \t\t\t\t}\n-\t\t\t\tcontents += subdir;\n+\n+\t\t\t\tcontents += read_directory_recursive(dir,\n+\t\t\t\t\tfullname, fullname, baselen + len, 0);\n \t\t\t\tcontinue;\n \t\t\tcase DT_REG:\n \t\t\tcase DT_LNK:\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\tadd_name(dir, fullname, baselen + len);\n \t\t\tcontents++;\n+\t\t\tif (check_only)\n+\t\t\t\tgoto exit_early;\n+\t\t\telse\n+\t\t\t\tadd_name(dir, fullname, baselen + len);\n \t\t}\n+exit_early:\n \t\tclosedir(fdir);\n \n \t\tpop_exclude_per_directory(dir, exclude_stk);\n@@ -393,7 +396,7 @@ int read_directory(struct dir_struct *di\n \t\t}\n \t}\n \n-\tread_directory_recursive(dir, path, base, baselen);\n+\tread_directory_recursive(dir, path, base, baselen, 0);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\n \treturn dir->nr;\n }\n"},{"id":"27869","messageId":"7v64f861t9.fsf@assigned-by-dhcp.cox.net","threadId":"5719","inReplyTo":"Pine.LNX.4.63.0609280240060.14200@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2nd try] runstatus: do not recurse into subdirectories if not needed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-28T04:40:50Z","receivedAt":"2006-09-28T04:40:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This speeds up the case when you run git-status, having an untracked\n> subdirectory containing huge amounts of files.\n>\n> Signed-off-by: Johannes Schindelin \n>\n> ---\n>\n> \tOn Thu, 28 Sep 2006, Johannes Schindelin wrote:\n>\n> \tOkay, so no more dir_is_empty(). Instead, read_directory_recursive()\n> \tgets a flag. With this flag, \"check_only\", it exits as soon as it\n> \tfound valid entries, but does not add any. Way easier.\n\nYeah, the logic is a lot easier to follow.\n"}]}