Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Deleting refs does not remove parent directories if they are empty.
> Empty directories add extra overhead to startup time of most of git
> commands because they have to traverse $GIT_DIR/refs.
Perhaps drop the first line and replace with the description of what you
do differently from the first round?
"git pack-refs" tries to remove directory that becomes empty but it
does not try to do so hard enough, leaving a parent directory full of
empty children directories without removing.
or something?
> Some directories are kept by this patch even if they are empty (refs,
> refs/heads and refs/tags). The first one is one of git repository
> signature. The rest is created by init-db, one may expect them to always
> be there.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> v2, no more refs code change.
>
> Part of the reason I do not want to update delete_ref() is because it
> won't remove empty directories in existing repositories.
While I agree with Peff that people would expect doing other things while
pack-refs is running would be much "riskier" and doing this inside
pack-refs is far more preferable than doing so during normal read-only
operation, I wonder why we would want a completely separate pass that
scans the entire hierarchy. Would it make more sense to note the
directory for which rmdir() fails in try_remove_empty_parents(), and
revisit only these directories, at least?
Wouldn't we want to rmdir() the corresponding logs/ hierarchy while at it
to be consistent?
>
> pack-refs.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/pack-refs.c b/pack-refs.c
> index f09a054..bb3a9c4 100644
> --- a/pack-refs.c
> +++ b/pack-refs.c
> @@ -91,6 +91,58 @@ static void try_remove_empty_parents(char *name)
> }
> }
>
> +static int prune_empty_dirs(const char *path)
> +{
> + int nr_entries = 0, pathlen = strlen(path);
> + DIR *dir;
> + struct dirent *de;
> + char *subpath;
> +
> + dir = opendir(git_path("%s", path));
> +
> + if (!dir)
> + return 0;
> +
> + subpath = xmalloc(pathlen + 257);
What is this 257 about?
> + memcpy(subpath, path, pathlen);
> + if (pathlen && path[pathlen-1] != '/')
> + subpath[pathlen++] = '/';
> +
> + while ((de = readdir(dir)) != NULL) {
> + struct stat st;
> + int namelen;
> +
> + if (de->d_name[0] == '.') {
> + if (strcmp(de->d_name, "..") && strcmp(de->d_name, "."))
> + nr_entries++;
> + continue;
> + }
> + nr_entries++;
> + namelen = strlen(de->d_name);
> + if (namelen > 255)
> + continue;
> + if (has_extension(de->d_name, ".lock"))
> + continue;
This is a sign that somebody else might be actively accessing this
repository.
> + memcpy(subpath + pathlen, de->d_name, namelen+1);
> + if (stat(git_path("%s", subpath), &st) < 0)
> + continue;
> + if (S_ISDIR(st.st_mode)) {
> + int removed = prune_empty_dirs(subpath);
> + if (removed)
> + nr_entries--;
> + continue;
> + }
> + }
> + free(subpath);
> + closedir(dir);
> + if (nr_entries == 0 &&
> + strcmp(path, "refs") &&
> + strcmp(path, "refs/heads") &&
> + strcmp(path, "refs/tags"))
> + return rmdir(git_path("%s", path)) == 0;
> + return 0;
> +}
> +
> /* make sure nobody touched the ref, and unlink */
> static void prune_ref(struct ref_to_prune *r)
> {
> @@ -109,6 +161,7 @@ static void prune_refs(struct ref_to_prune *r)
> prune_ref(r);
> r = r->next;
> }
> + prune_empty_dirs("refs");
> }
>
> static struct lock_file packed;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html