[PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev

[PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev

[Mike Hommey]

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


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

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

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> 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.