[PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev

Mike Hommey· Nov 26, 2014, 10:12 UTC · lore

An example where this happens is when doing an ls-tree on a tree that contains a commit link. In that case, find_unique_abbrev is called to get a non-abbreviated hex sha1, but still, a lookup is done as to whether the sha1 is in the repository (which ends up looking for a loose object in .git/objects), while the result of that lookup is not used when returning a non-abbreviated hex sha1.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 sha1_name.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This hit me on a corner case, where I kind of abuse commit links and have a tree with tens of thousands of them.

Without the patch:
  $ time git ls-tree -r HEAD | wc -l
  114987

  real	0m4.412s
  user	0m1.980s
  sys	0m2.480s

With the patch:
  $ time git ls-tree -r HEAD | wc -l
  114987

  real	0m0.205s
  user	0m0.196s
  sys	0m0.012s

Show changes to sha1_name.c +1 −1

diff --git a/sha1_name.c b/sha1_name.c
index 5b004f5..cb88170 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -372,10 +372,10 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 	int status, exists;
 	static char hex[41];
 
-	exists = has_sha1_file(sha1);
 	memcpy(hex, sha1_to_hex(sha1), 40);
 	if (len == 40 || !len)
 		return hex;
+	exists = has_sha1_file(sha1);
 	while (len < 40) {
 		unsigned char sha1_ret[20];
 		status = get_short_sha1(hex, len, sha1_ret, GET_SHA1_QUIETLY);

-- 
2.1.1

Junio C Hamano· Nov 26, 2014, 18:50 UTC · re: Mike Hommey · lore

Re: [PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev

Mike Hommey <mh@glandium.org> writes:

Show 6 quoted lines

> An example where this happens is when doing an ls-tree on a tree that
> contains a commit link. In that case, find_unique_abbrev is called
> to get a non-abbreviated hex sha1, but still, a lookup is done as
> to whether the sha1 is in the repository (which ends up looking for
> a loose object in .git/objects), while the result of that lookup is
> not used when returning a non-abbreviated hex sha1.

Makes sense. If we are not abbreviating, because the function is prepared to give answer for both missing and existing objects, there is no point checking the existence.

Thanks.

patchsha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev