git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 5/5] clean: improve performance when removing lots of directories

From
Erik Elfström <erik.elfstrom@gmail.com>
Date
Apr 26, 2015, 06:49 UTC
Message-ID
<1430030985-14499-6-git-send-email-erik.elfstrom@gmail.com>
In-Reply-To
<1430030985-14499-1-git-send-email-erik.elfstrom@gmail.com>

"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.

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>
---
 builtin/clean.c  | 26 ++++++++++++++++++++++----
 t/t7300-clean.sh |  8 +++-----
 2 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 98c103f..17a1c13 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -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"
@@ -148,6 +147,27 @@ 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 unused_error_code;
+	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, &unused_error_code) || is_git_directory(path->buf))
+		ret = 1;
+	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)
 {
@@ -155,13 +175,11 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	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),
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 11f3a6d..4d07a4a 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -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 &&
@@ -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 &&
@@ -501,7 +499,7 @@ test_expect_success 'should not clean 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 &&
@@ -523,7 +521,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 &&

-- 
2.4.0.rc3.8.gbb31afb
Previous: erik elfströmNext: Jeff King
Message 33 of 38 in “Improving performance of git clean”
  1. 0/5 Improving performance of git cleanErik Elfström, Apr 25, 2015
  2. 1/5 setup: add gentle version of read_gitfileErik Elfström, Apr 25, 2015
  3. Junio C HamanoApr 25, 2015
  4. Junio C HamanoApr 25, 2015
  5. 2/5 setup: sanity check file size in read_gitfile_gentlyErik Elfström, Apr 25, 2015
  6. Junio C HamanoApr 25, 2015
  7. Erik ElfströmApr 25, 2015
  8. Junio C HamanoApr 26, 2015
  9. 0/5 Improving performance of git cleanErik Elfström, Apr 26, 2015
  10. 1/5 setup: add gentle version of read_gitfileErik Elfström, Apr 26, 2015
  11. Jeff KingApr 28, 2015
  12. erik elfströmApr 28, 2015
  13. Jeff KingApr 28, 2015
  14. Jonathan NiederApr 28, 2015
  15. Jeff KingApr 28, 2015
  16. Jonathan NiederApr 28, 2015
  17. Jeff KingApr 28, 2015
  18. Jonathan NiederApr 28, 2015
  19. Junio C HamanoApr 28, 2015
  20. Stefan BellerApr 29, 2015
  21. Junio C HamanoApr 30, 2015
  22. 2/5 setup: sanity check file size in read_gitfile_gentlyErik Elfström, Apr 26, 2015
  23. Jeff KingApr 28, 2015
  24. Windows path limites, was Re: [PATCH v5 2/5] setup: sanity check file size in read_gitfile_gentlyJohannes Schindelin, Apr 28, 2015
  25. Doug KellyApr 28, 2015
  26. Johannes SchindelinApr 28, 2015
  27. erik elfströmApr 28, 2015
  28. Junio C HamanoApr 29, 2015
  29. 3/5 t7300: add tests to document behavior of clean and nested gitErik Elfström, Apr 26, 2015
  30. 4/5 p7300: add performance tests for cleanErik Elfström, Apr 26, 2015
  31. Jeff KingApr 28, 2015
  32. erik elfströmApr 28, 2015
  33. 5/5 clean: improve performance when removing lots of directoriesErik Elfström, Apr 26, 2015
  34. Jeff KingApr 28, 2015
  35. erik elfströmApr 28, 2015
  36. 3/5 t7300: add tests to document behavior of clean and nested gitErik Elfström, Apr 25, 2015
  37. 4/5 p7300: add performance tests for cleanErik Elfström, Apr 25, 2015
  38. 5/5 clean: improve performance when removing lots of directoriesErik Elfström, Apr 25, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.