patchpack-refs: remove newly empty directories

In a large repository which uses directories to organize many refs, "git pack-refs --all --prune" does not improve performance so much as it should, unless we remove all the now-empty directories as well.

Signed-off-by: Greg Price <price@ksplice.com>
---
 pack-refs.c          |    8 ++++++++
 t/t3210-pack-refs.sh |    6 ++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

Greg Price <price@ksplice.com> writes:

Will this keep refs/heads/p/q that is empty after packing p/q/r/s branch that happens to be the only branch whose name begins with p/?

I do not want a careless loop that will remove refs/heads after packing "master" that happens to be the only local branch, but still...

On Mon, Jul 5, 2010 at 11:02 PM, Junio C Hamano <gitster@pobox.com> wrote:

It will.  I could fix that with something like this (untested):

/* Remove empty parents, but spare refs/ and immediate subdirs.
   Note, munges *name. */
static void try_remove_empty_parents(char *name)
{
  char *p, *q;
  int i;
  p = name;
  for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
    while (*p && *p != '/')
      p++;
    if (*p)
      p++;
  }
  for (q = p; *q; q++)
    ;
  while (1) {
    for ( ; q > p && *q != '/'; q--)
      ;
    if (q == p)
      break;
    *q = '\0';
    if (rmdir(git_path("%s", name)))
      break;
  }
}

and then

      if (lock) {
              unlink_or_warn(git_path("%s", r->name));
              unlock_ref(lock);
+              try_remove_empty_parents(r->name);
      }

Sound reasonable?

Greg

In a large repository which uses directories to organize many refs, "git pack-refs --all --prune" does not improve performance so much as it should, unless we remove all the now-empty directories as well.

Signed-off-by: Greg Price <price@ksplice.com>
---
This version removes empty grandparent directories, etc, but always
leaves in place refs/heads/ and its siblings.

We also tolerate duplicate slashes in refnames, because check_ref_format() in refs.c does the same.

 pack-refs.c          |   30 ++++++++++++++++++++++++++++++
 t/t3210-pack-refs.sh |    6 ++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

Am 7/6/2010 0:27, schrieb Greg Price:

Before your patch, when you create a ref refs/heads/foo/bar, then pack refs, then it is impossible to create a ref refs/heads/foo because refs/heads/foo still exists as directory. And this is a good thing.

With your patch, is there any mechanism that inhibits that refs/heads/foo is created when directory refs/heads/foo does not exist, but a packed ref refs/heads/foo/bar is present?

-- Hannes

Johannes Sixt <j.sixt@viscovery.net> writes:

$ git grep -e is_refname_available refs.c

Greg Price <price@ksplice.com> writes:

What happens if a parallel running git wants to update a ref in one of the now-empty directories? Can it get a spurious error after it has called safe_create_leading_directories?

Andreas.

On Tue, Jul 6, 2010 at 2:49 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:

Good question! I asked the same question. =) It can get the same kind of error that happens if two parallel running git processes try to update the same ref.

Anything that tries to update a ref will call lock_ref_sha1_basic(), which calls safe_create_leading_directories() to make the directory and then hold_lock_file_for_update() to open(O_CREAT|O_EXCL) a lock file inside the directory. So if the prune happens after the open(), it will simply not remove the directory and all is well. If it happens between the safe_create_leading_directories() and the open(), then the open() will fail. This is also what happens if the lock has been taken by another git process updating the same ref. Currently lock_ref_sha1_basic() chooses to have the process die in this case and print an error message.

Similarly the prune could remove a directory just before safe_create_leading_directories() creates one inside it. Then safe_create_leading_directories() will fail and loc_ref_sha1_basic() will print an error message and return failure. In both cases, since we are exposed to a similar failure if a parallel process is updating the same ref (a common operation) rather than packing refs (an uncommon operation), I don't think this is a problem.

Greg