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

Re: [PATCH 0/6] address packed-refs speed regressions

From
René Scharfe <l.s.r@web.de>
Date
Apr 5, 2015, 13:41 UTC
Message-ID
<55213B93.9050207@web.de>
In-Reply-To
<20150405010611.GA15901@peff.net>
Am 05.04.2015 um 03:06 schrieb Jeff King:
Show 33 quoted lines
> As I've mentioned before, I have some repositories with rather large
> numbers of refs. The worst one has ~13 million refs, for a 1.6GB
> packed-refs file. So I was saddened by this:
> 
>    $ time git.v2.0.0 rev-parse refs/heads/foo >/dev/null 2>&1
>    real    0m6.840s
>    user    0m6.404s
>    sys     0m0.440s
> 
>    $ time git.v2.4.0-rc1 rev-parse refs/heads/foo >/dev/null 2>&1
>    real    0m19.432s
>    user    0m18.996s
>    sys     0m0.456s
> 
> The command isn't important; what I'm really measuring is loading the
> packed-refs file. And yes, of course this repository is absolutely
> ridiculous. But the slowdowns here are linear with the number of refs.
> So _every_ git command got a little bit slower, even in less crazy
> repositories. We just didn't notice it as much.
> 
> Here are the numbers after this series:
> 
>    real    0m8.539s
>    user    0m8.052s
>    sys     0m0.496s
> 
> Much better, but I'm frustrated that they are still 20% slower than the
> original.
> 
> The main culprits seem to be d0f810f (which introduced some extra
> expensive code for each ref) and my 10c497a, which switched from fgets()
> to strbuf_getwholeline. It turns out that strbuf_getwholeline is really
> slow.

10c497a changed read_packed_refs(), which reads *all* packed refs. Each is checked for validity. That sounds expensive if the goal is just to look up a single (non-existing) ref.

Would it help to defer any checks until a ref is actually accessed? Can a binary search be used instead of reading the whole file?

I wonder if pluggable reference backends could help here. Storing refs in a database table indexed by refname should simplify things.

Short-term, can we avoid the getc()/strbuf_grow() dance e.g. by mapping the packed refs file? What numbers do you get with the following patch?

---
 refs.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 47e4e53..144255f 100644
--- a/refs.c
+++ b/refs.c
@@ -1153,16 +1153,35 @@ static const char *parse_ref_line(struct strbuf *line, unsigned char *sha1)
  *      compatibility with older clients, but we do not require it
  *      (i.e., "peeled" is a no-op if "fully-peeled" is set).
  */
-static void read_packed_refs(FILE *f, struct ref_dir *dir)
+static void read_packed_refs(int fd, struct ref_dir *dir)
 {
 	struct ref_entry *last = NULL;
 	struct strbuf line = STRBUF_INIT;
 	enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;
+	struct stat st;
+	void *map;
+	size_t mapsz, len;
+	const char *p;
+
+	fstat(fd, &st);
+	mapsz = xsize_t(st.st_size);
+	if (!mapsz)
+		return;
+	map = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);
 
-	while (strbuf_getwholeline(&line, f, '\n') != EOF) {
+	for (p = map, len = mapsz; len; ) {
 		unsigned char sha1[20];
 		const char *refname;
 		const char *traits;
+		const char *nl;
+		size_t linelen;
+
+		nl = memchr(p, '\n', len);
+		linelen = nl ? nl - p + 1 : len;
+		strbuf_reset(&line);
+		strbuf_add(&line, p, linelen);
+		p += linelen;
+		len -= linelen;
 
 		if (skip_prefix(line.buf, "# pack-refs with:", &traits)) {
 			if (strstr(traits, " fully-peeled "))
@@ -1204,6 +1223,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
 	}
 
 	strbuf_release(&line);
+	munmap(map, mapsz);
 }
 
 /*
@@ -1224,16 +1244,16 @@ static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
 		clear_packed_ref_cache(refs);
 
 	if (!refs->packed) {
-		FILE *f;
+		int fd;
 
 		refs->packed = xcalloc(1, sizeof(*refs->packed));
 		acquire_packed_ref_cache(refs->packed);
 		refs->packed->root = create_dir_entry(refs, "", 0, 0);
-		f = fopen(packed_refs_file, "r");
-		if (f) {
-			stat_validity_update(&refs->packed->validity, fileno(f));
-			read_packed_refs(f, get_ref_dir(refs->packed->root));
-			fclose(f);
+		fd = open(packed_refs_file, O_RDONLY);
+		if (fd >= 0) {
+			stat_validity_update(&refs->packed->validity, fd);
+			read_packed_refs(fd, get_ref_dir(refs->packed->root));
+			close(fd);
 		}
 	}
 	return refs->packed;

-- 
2.3.5
Previous: Jeff KingNext: Jeff King
Message 21 of 44 in “address packed-refs speed regressions”
  1. 0/6 address packed-refs speed regressionsJeff King, Apr 5, 2015
  2. 1/6 strbuf_getwholeline: use getc macroJeff King, Apr 5, 2015
  3. 2/6 git-compat-util: add fallbacks for unlocked stdioJeff King, Apr 5, 2015
  4. 3/6 strbuf_getwholeline: use getc_unlockedJeff King, Apr 5, 2015
  5. Jeff KingApr 5, 2015
  6. Jeff KingApr 5, 2015
  7. Jeff KingApr 5, 2015
  8. Junio C HamanoApr 5, 2015
  9. Duy NguyenApr 5, 2015
  10. Jeff KingApr 5, 2015
  11. Junio C HamanoApr 5, 2015
  12. Rasmus VillemoesApr 7, 2015
  13. Jeff KingApr 7, 2015
  14. Rasmus VillemoesApr 7, 2015
  15. Jeff KingApr 8, 2015
  16. 4/6 strbuf: add an optimized 1-character strbuf_growJeff King, Apr 5, 2015
  17. Eric SunshineApr 6, 2015
  18. Jeff KingApr 6, 2015
  19. 5/6 t1430: add another refs-escape testJeff King, Apr 5, 2015
  20. 6/6 refname_is_safe: avoid expensive normalize_path_copy callJeff King, Apr 5, 2015
  21. René ScharfeApr 5, 2015
  22. Jeff KingApr 5, 2015
  23. Jeff KingApr 5, 2015
  24. René ScharfeApr 5, 2015
  25. René ScharfeApr 5, 2015
  26. Jeff KingApr 6, 2015
  27. 0/9 address packed-refs speed regressionsJeff King, Apr 16, 2015
  28. 1/9 strbuf_getwholeline: use getc macroJeff King, Apr 16, 2015
  29. 2/9 git-compat-util: add fallbacks for unlocked stdioJeff King, Apr 16, 2015
  30. 3/9 strbuf_getwholeline: use getc_unlockedJeff King, Apr 16, 2015
  31. 4/9 config: use getc_unlocked when reading from fileJeff King, Apr 16, 2015
  32. 5/9 strbuf_addch: avoid calling strbuf_growJeff King, Apr 16, 2015
  33. 6/9 strbuf_getwholeline: avoid calling strbuf_growJeff King, Apr 16, 2015
  34. 7/9 strbuf_getwholeline: use getdelim if it is availableJeff King, Apr 16, 2015
  35. Eric SunshineApr 17, 2015
  36. Jeff KingApr 21, 2015
  37. Eric SunshineMay 8, 2015
  38. Jeff KingMay 9, 2015
  39. Eric SunshineJun 2, 2015
  40. Johannes SchindelinApr 22, 2015
  41. Jeff KingApr 22, 2015
  42. 8/9 read_packed_refs: avoid double-checking sane refsJeff King, Apr 16, 2015
  43. 9/9 t1430: add another refs-escape testJeff King, Apr 16, 2015
  44. Jeff KingApr 16, 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.