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

[PATCH/RFC v1 1/6] symlinks.c: small cleanup and optimisation

From
KBKjetil Barvik <barvik@broadpark.no>
Date
Jan 26, 2009, 21:17 UTC
Message-ID
<1233004637-15112-2-git-send-email-barvik@broadpark.no>
In-Reply-To
<1233004637-15112-1-git-send-email-barvik@broadpark.no>

Simplify the if-else test in longest_match_lstat_cache() such that we only have one simple if test. Instead of testing for 'i == cache.len' or 'i == len', we transform this to a common test for 'i == max_len'.

And to further optimise we use 'i >= max_len' instead of 'i == max_len', the reason is that it is now the exact opposite of one part inside the while-loop termination expression 'i < max_len && name[i] == cache.path[i]', and then the compiler can hopefully/probably reuse a test-result from it.

We do similar transformations inside the lstat_cache() and the invalidate_lstat_cache() functions.

The result is a little smaller text-size as shown below:
 ~/git/git $ size symlinks_??_*
   text	   data	    bss	    dec	    hex	filename
   1282       0	   4116	   5398	   1516	symlinks_O2_after.o
   1378       0	   4116	   5494	   1576	symlinks_O2_before.o
    896       0	   4116	   5012	   1394	symlinks_Os_after.o
    902       0	   4116	   5018	   139a	symlinks_Os_before.o

'O2' means that the file is compiled with 'gcc -O2', and similar 'Os' means that the file is compiled with 'gcc -Os' (gcc 4.3.2).

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 symlinks.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/symlinks.c b/symlinks.c
index f262b7c..81f490c 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -25,15 +25,16 @@ static inline int longest_match_lstat_cache(int len, const char *name,
 		}
 		i++;
 	}
-	/* Is the cached path string a substring of 'name'? */
-	if (i == cache.len && cache.len < len && name[cache.len] == '/') {
-		match_len_prev = match_len;
-		match_len = cache.len;
-	/* Is 'name' a substring of the cached path string? */
-	} else if ((i == len && len < cache.len && cache.path[len] == '/') ||
-		   (i == len && len == cache.len)) {
+	/*
+	 * Is the cached path string a substring of 'name', is 'name'
+	 * a substring of the cached path string, or is 'name' and the
+	 * cached path string the exact same string?
+	 */
+	if (i >= max_len && ((i < len && name[i] == '/') ||
+			     (i < cache.len && cache.path[i] == '/') ||
+			     (len == cache.len))) {
 		match_len_prev = match_len;
-		match_len = len;
+		match_len = i;
 	}
 	*previous_slash = match_len_prev;
 	return match_len;
@@ -91,7 +92,7 @@ static int lstat_cache(int len, const char *name,
 		match_len = last_slash =
 			longest_match_lstat_cache(len, name, &previous_slash);
 		match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK);
-		if (match_flags && match_len == cache.len)
+		if (match_flags && match_len >= cache.len)
 			return match_flags;
 		/*
 		 * If we now have match_len > 0, we would know that
@@ -102,7 +103,7 @@ static int lstat_cache(int len, const char *name,
 		 * we can return immediately.
 		 */
 		match_flags = track_flags & FL_DIR;
-		if (match_flags && len == match_len)
+		if (match_flags && match_len >= len)
 			return match_flags;
 	}
 
@@ -153,7 +154,7 @@ static int lstat_cache(int len, const char *name,
 		cache.path[last_slash] = '\0';
 		cache.len = last_slash;
 		cache.flags = save_flags;
-	} else if (track_flags & FL_DIR &&
+	} else if ((track_flags & FL_DIR) &&
 		   last_slash_dir > 0 && last_slash_dir <= PATH_MAX) {
 		/*
 		 * We have a separate test for the directory case,
@@ -184,7 +185,7 @@ void invalidate_lstat_cache(int len, const char *name)
 	int match_len, previous_slash;
 
 	match_len = longest_match_lstat_cache(len, name, &previous_slash);
-	if (len == match_len) {
+	if (match_len >= len) {
 		if ((cache.track_flags & FL_DIR) && previous_slash > 0) {
 			cache.path[previous_slash] = '\0';
 			cache.len = previous_slash;
-- 
1.6.1.349.g99fa5
Previous: Kjetil BarvikNext: Junio C Hamano
Message 2 of 16 in “git checkout: more cleanups, optimisation, less lstat() calls”
  1. 0/6 git checkout: more cleanups, optimisation, less lstat() callsKjetil Barvik, Jan 26, 2009
  2. 1/6 symlinks.c: small cleanup and optimisationKjetil Barvik, Jan 26, 2009
  3. ??? Re: [PATCH/RFC v1 1/6] symlinks.c: small cleanup and optimisationJunio C Hamano, Jan 28, 2009
  4. Kjetil BarvikJan 29, 2009
  5. 2/6 remove some memcpy() and strchr() calls inside create_directories()Kjetil Barvik, Jan 26, 2009
  6. Junio C HamanoJan 28, 2009
  7. 3/6 cleanup of write_entry() in entry.cKjetil Barvik, Jan 26, 2009
  8. Junio C HamanoJan 28, 2009
  9. 4/6 use fstat() instead of lstat() when we have an opened fileKjetil Barvik, Jan 26, 2009
  10. 5/6 combine-diff.c: remove a call to fstat() inside show_patch_diff()Kjetil Barvik, Jan 26, 2009
  11. Mike RalphsonJan 27, 2009
  12. Kjetil BarvikJan 27, 2009
  13. Mike RalphsonJan 27, 2009
  14. Junio C HamanoJan 28, 2009
  15. Junio C HamanoJan 29, 2009
  16. Kjetil BarvikJan 29, 2009

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.