Merge branch 'rs/sha1-name-readdir-optim'

Optimize "what are the object names already taken in an alternate
object database?" query that is used to derive the length of prefix
an object name is uniquely abbreviated to.

* rs/sha1-name-readdir-optim:
  sha1_file: guard against invalid loose subdirectory numbers
  sha1_file: let for_each_file_in_obj_subdir() handle subdir names
  p4205: add perf test script for pretty log formats
  sha1_name: cache readdir(3) results in find_short_object_filename()

builtin/fsck.c

@@ -537,7 +537,7 @@ static int fsck_cruft(const char *basename, const char *path, void *data)
 	return 0;
 }
 
-static int fsck_subdir(int nr, const char *path, void *progress)
+static int fsck_subdir(unsigned int nr, const char *path, void *progress)
 {
 	display_progress(progress, nr + 1);
 	return 0;

builtin/prune-packed.c

@@ -10,7 +10,7 @@ static const char * const prune_packed_usage[] = {
 
 static struct progress *progress;
 
-static int prune_subdir(int nr, const char *path, void *data)
+static int prune_subdir(unsigned int nr, const char *path, void *data)
 {
 	int *opts = data;
 	display_progress(progress, nr + 1);

builtin/prune.c

@@ -68,7 +68,7 @@ static int prune_cruft(const char *basename, const char *path, void *data)
 	return 0;
 }
 
-static int prune_subdir(int nr, const char *path, void *data)
+static int prune_subdir(unsigned int nr, const char *path, void *data)
 {
 	if (!show_only)
 		rmdir(path);

cache.h

@@ -12,6 +12,7 @@
 #include "pack-revindex.h"
 #include "hash.h"
 #include "path.h"
+#include "sha1-array.h"
 
 #ifndef platform_SHA_CTX
 /*
@@ -1540,6 +1541,16 @@ extern struct alternate_object_database {
 	struct strbuf scratch;
 	size_t base_len;
 
+	/*
+	 * Used to store the results of readdir(3) calls when searching
+	 * for unique abbreviated hashes.  This cache is never
+	 * invalidated, thus it's racy and not necessarily accurate.
+	 * That's fine for its purpose; don't use it for tasks requiring
+	 * greater accuracy!
+	 */
+	char loose_objects_subdir_seen[256];
+	struct oid_array loose_objects_cache;
+
 	char path[FLEX_ARRAY];
 } *alt_odb_list;
 extern void prepare_alt_odb(void);
@@ -1755,9 +1766,15 @@ typedef int each_loose_object_fn(const struct object_id *oid,
 typedef int each_loose_cruft_fn(const char *basename,
 				const char *path,
 				void *data);
-typedef int each_loose_subdir_fn(int nr,
+typedef int each_loose_subdir_fn(unsigned int nr,
 				 const char *path,
 				 void *data);
+int for_each_file_in_obj_subdir(unsigned int subdir_nr,
+				struct strbuf *path,
+				each_loose_object_fn obj_cb,
+				each_loose_cruft_fn cruft_cb,
+				each_loose_subdir_fn subdir_cb,
+				void *data);
 int for_each_loose_file_in_objdir(const char *path,
 				  each_loose_object_fn obj_cb,
 				  each_loose_cruft_fn cruft_cb,

sha1_file.c

@@ -3735,22 +3735,32 @@ void assert_sha1_type(const unsigned char *sha1, enum object_type expect)
 		    typename(expect));
 }
 
-static int for_each_file_in_obj_subdir(int subdir_nr,
+int for_each_file_in_obj_subdir(unsigned int subdir_nr,
-				       struct strbuf *path,
+				struct strbuf *path,
-				       each_loose_object_fn obj_cb,
+				each_loose_object_fn obj_cb,
-				       each_loose_cruft_fn cruft_cb,
+				each_loose_cruft_fn cruft_cb,
-				       each_loose_subdir_fn subdir_cb,
+				each_loose_subdir_fn subdir_cb,
-				       void *data)
+				void *data)
 {
-	size_t baselen = path->len;
+	size_t origlen, baselen;
-	DIR *dir = opendir(path->buf);
+	DIR *dir;
 	struct dirent *de;
 	int r = 0;
 
+	if (subdir_nr > 0xff)
+		BUG("invalid loose object subdirectory: %x", subdir_nr);
+
+	origlen = path->len;
+	strbuf_complete(path, '/');
+	strbuf_addf(path, "%02x", subdir_nr);
+	baselen = path->len;
+
+	dir = opendir(path->buf);
 	if (!dir) {
-		if (errno == ENOENT)
+		if (errno != ENOENT)
-			return 0;
+			r = error_errno("unable to open %s", path->buf);
-		return error_errno("unable to open %s", path->buf);
+		strbuf_setlen(path, origlen);
+		return r;
 	}
 
 	while ((de = readdir(dir))) {
@@ -3788,6 +3798,8 @@ static int for_each_file_in_obj_subdir(int subdir_nr,
 	if (!r && subdir_cb)
 		r = subdir_cb(subdir_nr, path->buf, data);
 
+	strbuf_setlen(path, origlen);
+
 	return r;
 }
 
@@ -3797,15 +3809,12 @@ int for_each_loose_file_in_objdir_buf(struct strbuf *path,
 			    each_loose_subdir_fn subdir_cb,
 			    void *data)
 {
-	size_t baselen = path->len;
 	int r = 0;
 	int i;
 
 	for (i = 0; i < 256; i++) {
-		strbuf_addf(path, "/%02x", i);
 		r = for_each_file_in_obj_subdir(i, path, obj_cb, cruft_cb,
 						subdir_cb, data);
-		strbuf_setlen(path, baselen);
 		if (r)
 			break;
 	}

sha1_name.c

@@ -78,10 +78,19 @@ static void update_candidates(struct disambiguate_state *ds, const struct object
 	/* otherwise, current can be discarded and candidate is still good */
 }
 
+static int append_loose_object(const struct object_id *oid, const char *path,
+			       void *data)
+{
+	oid_array_append(data, oid);
+	return 0;
+}
+
+static int match_sha(unsigned, const unsigned char *, const unsigned char *);
+
 static void find_short_object_filename(struct disambiguate_state *ds)
 {
+	int subdir_nr = ds->bin_pfx.hash[0];
 	struct alternate_object_database *alt;
-	char hex[GIT_MAX_HEXSZ];
 	static struct alternate_object_database *fakeent;
 
 	if (!fakeent) {
@@ -96,29 +105,29 @@ static void find_short_object_filename(struct disambiguate_state *ds)
 	}
 	fakeent->next = alt_odb_list;
 
-	xsnprintf(hex, sizeof(hex), "%.2s", ds->hex_pfx);
 	for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
-		struct strbuf *buf = alt_scratch_buf(alt);
+		int pos;
-		struct dirent *de;
-		DIR *dir;
 
-		strbuf_addf(buf, "%.2s/", ds->hex_pfx);
+		if (!alt->loose_objects_subdir_seen[subdir_nr]) {
-		dir = opendir(buf->buf);
+			struct strbuf *buf = alt_scratch_buf(alt);
-		if (!dir)
+			for_each_file_in_obj_subdir(subdir_nr, buf,
-			continue;
+						    append_loose_object,
-
+						    NULL, NULL,
-		while (!ds->ambiguous && (de = readdir(dir)) != NULL) {
+						    &alt->loose_objects_cache);
-			struct object_id oid;
+			alt->loose_objects_subdir_seen[subdir_nr] = 1;
-
+		}
-			if (strlen(de->d_name) != GIT_SHA1_HEXSZ - 2)
+
-				continue;
+		pos = oid_array_lookup(&alt->loose_objects_cache, &ds->bin_pfx);
-			if (memcmp(de->d_name, ds->hex_pfx + 2, ds->len - 2))
+		if (pos < 0)
-				continue;
+			pos = -1 - pos;
-			memcpy(hex + 2, de->d_name, GIT_SHA1_HEXSZ - 2);
+		while (!ds->ambiguous && pos < alt->loose_objects_cache.nr) {
-			if (!get_oid_hex(hex, &oid))
+			const struct object_id *oid;
-				update_candidates(ds, &oid);
+			oid = alt->loose_objects_cache.oid + pos;
+			if (!match_sha(ds->len, ds->bin_pfx.hash, oid->hash))
+				break;
+			update_candidates(ds, oid);
+			pos++;
 		}
-		closedir(dir);
 	}
 }
 

t/perf/p4205-log-pretty-formats.sh

@@ -0,0 +1,16 @@
+#!/bin/sh
+
+test_description='Tests the performance of various pretty format placeholders'
+
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+for format in %H %h %T %t %P %p %h-%h-%h
+do
+	test_perf "log with $format" "
+		git log --format=\"$format\" >/dev/null
+	"
+done
+
+test_done