Skip to content

Commit

Permalink
clean: improve performance when removing lots of directories
Browse files Browse the repository at this point in the history
"git clean" uses resolve_gitlink_ref() to check for the presence of
nested git repositories, but it has the drawback of creating a
ref_cache entry for every directory that should potentially be
cleaned. The linear search through the ref_cache list causes a massive
performance hit for large number of directories.

Modify clean.c:remove_dirs to use setup.c:is_git_directory and
setup.c:read_gitfile_gently instead.

Both these functions will open files and parse contents when they find
something that looks like a git repository. This is ok from a
performance standpoint since finding repository candidates should be
comparatively rare.

Using is_git_directory and read_gitfile_gently should give a more
standardized check for what is and what isn't a git repository but
also gives three behavioral changes.

The first change is that we will now detect and avoid cleaning empty
nested git repositories (only init run). This is desirable.

Second, we will no longer die when cleaning a file named ".git" with
garbage content (it will be cleaned instead). This is also desirable.

The last change is that we will detect and avoid cleaning empty bare
repositories that have been placed in a directory named ".git". This
is not desirable but should have no real user impact since we already
fail to clean non-empty bare repositories in the same scenario. This
is thus deemed acceptable.

On top of this we add some extra precautions. If read_gitfile_gently
fails to open the git file, read the git file or verify the path in
the git file we assume that the path with the git file is a valid
repository and avoid cleaning.

Update t7300 to reflect these changes in behavior.

The time to clean an untracked directory containing 100000 sub
directories went from 61s to 1.7s after this change.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Erik Elfström <erik.elfstrom@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Erik Elfström authored and Junio C Hamano committed Jun 15, 2015
1 parent f49a565 commit 0179ca7
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 10 deletions.
30 changes: 26 additions & 4 deletions builtin/clean.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "cache.h"
#include "dir.h"
#include "parse-options.h"
#include "refs.h"
#include "string-list.h"
#include "quote.h"
#include "column.h"
Expand Down Expand Up @@ -148,20 +147,43 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)
return 0;
}

/*
* Return 1 if the given path is the root of a git repository or
* submodule else 0. Will not return 1 for bare repositories with the
* exception of creating a bare repository in "foo/.git" and calling
* is_git_repository("foo").
*/
static int is_git_repository(struct strbuf *path)
{
int ret = 0;
int gitfile_error;
size_t orig_path_len = path->len;
assert(orig_path_len != 0);
if (path->buf[orig_path_len - 1] != '/')
strbuf_addch(path, '/');
strbuf_addstr(path, ".git");
if (read_gitfile_gently(path->buf, &gitfile_error) || is_git_directory(path->buf))
ret = 1;
if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||
gitfile_error == READ_GITFILE_ERR_READ_FAILED)
ret = 1; /* This could be a real .git file, take the
* safe option and avoid cleaning */
strbuf_setlen(path, orig_path_len);
return ret;
}

static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
int dry_run, int quiet, int *dir_gone)
{
DIR *dir;
struct strbuf quoted = STRBUF_INIT;
struct dirent *e;
int res = 0, ret = 0, gone = 1, original_len = path->len, len;
unsigned char submodule_head[20];
struct string_list dels = STRING_LIST_INIT_DUP;

*dir_gone = 1;

if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
!resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) && is_git_repository(path)) {
if (!quiet) {
quote_path_relative(path->buf, prefix, &quoted);
printf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),
Expand Down
10 changes: 4 additions & 6 deletions t/t7300-clean.sh
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ test_expect_success 'nested git work tree' '
! test -d bar
'

test_expect_failure 'should clean things that almost look like git but are not' '
test_expect_success 'should clean things that almost look like git but are not' '
rm -fr almost_git almost_bare_git almost_submodule &&
mkdir -p almost_git/.git/objects &&
mkdir -p almost_git/.git/refs &&
Expand All @@ -468,8 +468,6 @@ test_expect_failure 'should clean things that almost look like git but are not'
garbage
EOF
test_when_finished "rm -rf almost_*" &&
## This will fail due to die("Invalid gitfile format: %s", path); in
## setup.c:read_gitfile.
git clean -f -d &&
test_path_is_missing almost_git &&
test_path_is_missing almost_bare_git &&
Expand Down Expand Up @@ -501,7 +499,7 @@ test_expect_success 'should not clean submodules' '
test_path_is_missing to_clean
'

test_expect_failure 'should avoid cleaning possible submodules' '
test_expect_success 'should avoid cleaning possible submodules' '
rm -fr to_clean possible_sub1 &&
mkdir to_clean possible_sub1 &&
test_when_finished "rm -rf possible_sub*" &&
Expand All @@ -515,7 +513,7 @@ test_expect_failure 'should avoid cleaning possible submodules' '
test_path_is_missing to_clean
'

test_expect_failure 'nested (empty) git should be kept' '
test_expect_success 'nested (empty) git should be kept' '
rm -fr empty_repo to_clean &&
git init empty_repo &&
mkdir to_clean &&
Expand All @@ -537,7 +535,7 @@ test_expect_success 'nested bare repositories should be cleaned' '
test_path_is_missing subdir
'

test_expect_success 'nested (empty) bare repositories should be cleaned even when in .git' '
test_expect_failure 'nested (empty) bare repositories should be cleaned even when in .git' '
rm -fr strange_bare &&
mkdir strange_bare &&
git init --bare strange_bare/.git &&
Expand Down

0 comments on commit 0179ca7

Please sign in to comment.