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

[PATCH 7/9] strbuf_getwholeline: use getdelim if it is available

From
Jeff King <peff@peff.net>
Date
Apr 16, 2015, 09:01 UTC
Message-ID
<20150416090138.GG17938@peff.net>
In-Reply-To
<20150416084733.GA17811@peff.net>

We spend a lot of time in strbuf_getwholeline in a tight loop reading characters from a stdio handle into a buffer. The libc getdelim() function can do this for us with less overhead. It's in POSIX.1-2008, and was a GNU extension before that. Therefore we can't rely on it, but can fall back to the existing getc loop when it is not available.

The HAVE_GETDELIM knob is turned on automatically for Linux, where we have glibc. We don't need to set any new feature-test macros, because we already define _GNU_SOURCE. Other systems that implement getdelim may need to other macros (probably _POSIX_C_SOURCE >= 200809L), but we can address that along with setting the Makefile knob after testing the feature on those systems.

Running "git rev-parse refs/heads/does-not-exist" on a repo with an extremely large (1.6GB) packed-refs file went from (best-of-5):

  real    0m8.601s
  user    0m8.084s
  sys     0m0.524s
to:
  real    0m6.768s
  user    0m6.340s
  sys     0m0.432s
for a wall-clock speedup of 21%.
Based on a patch from Rasmus Villemoes <rv@rasmusvillemoes.dk>.
Signed-off-by: Jeff King <peff@peff.net>
---
If somebody has a FreeBSD or OS X system to test on, I'd
love to see what is needed to compile with HAVE_GETDELIM
there. And to confirm that the performance is much better.
Sharing my 1.6GB packed-refs file would be hard, but you
should be able to generate something large and ridiculous.
I'll leave that as an exercise to the reader.
 Makefile         |  6 ++++++
 config.mak.uname |  1 +
 strbuf.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)
diff --git a/Makefile b/Makefile
index 5f3987f..36655d5 100644
--- a/Makefile
+++ b/Makefile
@@ -359,6 +359,8 @@ all::
 # compiler is detected to support it.
 #
 # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
+#
+# Define HAVE_GETDELIM if your system has the getdelim() function.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1437,6 +1439,10 @@ ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
 
+ifdef HAVE_GETDELIM
+	BASIC_CFLAGS += -DHAVE_GETDELIM
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
diff --git a/config.mak.uname b/config.mak.uname
index f4e77cb..d26665f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -36,6 +36,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_DEV_TTY = YesPlease
 	HAVE_CLOCK_GETTIME = YesPlease
 	HAVE_CLOCK_MONOTONIC = YesPlease
+	HAVE_GETDELIM = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_ALLOCA_H = YesPlease
diff --git a/strbuf.c b/strbuf.c
index 921619e..0d4f4e5 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -435,6 +435,47 @@ int strbuf_getcwd(struct strbuf *sb)
 	return -1;
 }
 
+#ifdef HAVE_GETDELIM
+int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
+{
+	ssize_t r;
+
+	if (feof(fp))
+		return EOF;
+
+	strbuf_reset(sb);
+
+	/* Translate slopbuf to NULL, as we cannot call realloc on it */
+	if (!sb->alloc)
+		sb->buf = NULL;
+	r = getdelim(&sb->buf, &sb->alloc, term, fp);
+
+	if (r > 0) {
+		sb->len = r;
+		return 0;
+	}
+	assert(r == -1);
+
+	/*
+	 * Normally we would have called xrealloc, which will try to free
+	 * memory and recover. But we have no way to tell getdelim() to do so.
+	 * Worse, we cannot try to recover ENOMEM ourselves, because we have
+	 * no idea how many bytes were read by getdelim.
+	 *
+	 * Dying here is reasonable. It mirrors what xrealloc would do on
+	 * catastrophic memory failure. We skip the opportunity to free pack
+	 * memory and retry, but that's unlikely to help for a malloc small
+	 * enough to hold a single line of input, anyway.
+	 */
+	if (errno == ENOMEM)
+		die("Out of memory, getdelim failed");
+
+	/* Restore slopbuf that we moved out of the way before */
+	if (!sb->buf)
+		strbuf_init(sb, 0);
+	return EOF;
+}
+#else
 int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 {
 	int ch;
@@ -458,6 +499,7 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 	sb->buf[sb->len] = '\0';
 	return 0;
 }
+#endif
 
 int strbuf_getline(struct strbuf *sb, FILE *fp, int term)
 {

-- 
2.4.0.rc2.384.g7297a4a
Previous: Jeff KingNext: Eric Sunshine
Message 34 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.