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

[PATCH 3/3] --format=pretty: avoid calculating expensive expansions twice

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
Nov 10, 2007, 11:18 UTC
Message-ID
<47359382.1010600@lsrfire.ath.cx>
In-Reply-To
<20071110004635.GA14992@sigill.intra.peff.net>

As Jeff King remarked, format strings with duplicate placeholders can be slow to expand, because each instance is calculated anew.

This patch makes use of the fact that format_commit_message() and its helper functions only ever add stuff to the end of the strbuf. For certain expensive placeholders, store the offset and length of their expansion with the strbuf at the first occurrence. Later they expansion result can simply be copied from there -- no malloc() or strdup() required.

These certain placeholders are the abbreviated commit, tree and parent hashes, as the search for a unique abbreviated hash is quite costly. Here are the times for next (best of three runs):

$ time git log --pretty=format:%h >/dev/null

real 0m0.611s user 0m0.404s sys 0m0.204s

$ time git log --pretty=format:%h%h%h%h >/dev/null

real 0m1.206s user 0m0.744s sys 0m0.452s

And here those with this patch (and the previous two); the speedup of the single placeholder case is just noise:

$ time git log --pretty=format:%h >/dev/null

real 0m0.608s user 0m0.416s sys 0m0.192s

$ time git log --pretty=format:%h%h%h%h >/dev/null

real 0m0.639s user 0m0.488s sys 0m0.140s

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 pretty.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/pretty.c b/pretty.c
index 0c2f83b..ab142b8 100644
--- a/pretty.c
+++ b/pretty.c
@@ -369,8 +369,30 @@ struct format_commit_context {
 	struct chunk committer;
 	struct chunk encoding;
 	size_t body_off;
+
+	/* The following ones are relative to the result struct strbuf. */
+	struct chunk abbrev_commit_hash;
+	struct chunk abbrev_tree_hash;
+	struct chunk abbrev_parent_hashes;
 };
 
+static int add_again(struct strbuf *sb, struct chunk *chunk)
+{
+	if (chunk->len) {
+		strbuf_adddup(sb, chunk->off, chunk->len);
+		return 1;
+	}
+
+	/*
+	 * We haven't seen this chunk before.  Our caller is surely
+	 * going to add it the hard way now.  Remember the most likely
+	 * start of the to-be-added chunk: the current end of the
+	 * struct strbuf.
+	 */
+	chunk->off = sb->len;
+	return 0;
+}
+
 static void parse_commit_header(struct format_commit_context *context)
 {
 	const char *msg = context->commit->buffer;
@@ -447,15 +469,21 @@ static void format_commit_item(struct strbuf *sb, const char *placeholder,
 		strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
 		return;
 	case 'h':		/* abbreviated commit hash */
+		if (add_again(sb, &c->abbrev_commit_hash))
+			return;
 		strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
 		                                     DEFAULT_ABBREV));
+		c->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;
 		return;
 	case 'T':		/* tree hash */
 		strbuf_addstr(sb, sha1_to_hex(commit->tree->object.sha1));
 		return;
 	case 't':		/* abbreviated tree hash */
+		if (add_again(sb, &c->abbrev_tree_hash))
+			return;
 		strbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,
 		                                     DEFAULT_ABBREV));
+		c->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;
 		return;
 	case 'P':		/* parent hashes */
 		for (p = commit->parents; p; p = p->next) {
@@ -465,12 +493,16 @@ static void format_commit_item(struct strbuf *sb, const char *placeholder,
 		}
 		return;
 	case 'p':		/* abbreviated parent hashes */
+		if (add_again(sb, &c->abbrev_parent_hashes))
+			return;
 		for (p = commit->parents; p; p = p->next) {
 			if (p != commit->parents)
 				strbuf_addch(sb, ' ');
 			strbuf_addstr(sb, find_unique_abbrev(
 					p->item->object.sha1, DEFAULT_ABBREV));
 		}
+		c->abbrev_parent_hashes.len = sb->len -
+		                              c->abbrev_parent_hashes.off;
 		return;
 	case 'm':		/* left/right/bottom */
 		strbuf_addch(sb, (commit->object.flags & BOUNDARY)
-- 
1.5.3.5.1651.g30bf
Previous: René ScharfeNext: Junio C Hamano
Message 19 of 24 in “--pretty=format: on-demand format expansion”
  1. 2/2 --pretty=format: on-demand format expansionRené Scharfe, Nov 9, 2007
  2. Johannes SchindelinNov 9, 2007
  3. René ScharfeNov 9, 2007
  4. Johannes SchindelinNov 9, 2007
  5. Junio C HamanoNov 9, 2007
  6. Jeff KingNov 9, 2007
  7. René ScharfeNov 9, 2007
  8. Johannes SchindelinNov 10, 2007
  9. Jeff KingNov 10, 2007
  10. Jeff KingNov 10, 2007
  11. René ScharfeNov 10, 2007
  12. Johannes SchindelinNov 10, 2007
  13. René ScharfeNov 10, 2007
  14. Jeff KingNov 10, 2007
  15. Jeff KingNov 10, 2007
  16. Jeff KingNov 11, 2007
  17. 1/3 --pretty=format: parse commit message only onceRené Scharfe, Nov 10, 2007
  18. 2/3 add strbuf_adddup()René Scharfe, Nov 10, 2007
  19. 3/3 --format=pretty: avoid calculating expensive expansions twiceRené Scharfe, Nov 10, 2007
  20. Junio C HamanoNov 11, 2007
  21. Jeff KingNov 9, 2007
  22. René ScharfeNov 9, 2007
  23. Jeff KingNov 10, 2007
  24. Paul MackerrasNov 9, 2007

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.