{"thread":{"id":"38053","subject":"[PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev","startedAt":"2014-11-26T10:12:47Z","lastAt":"2014-11-26T18:50:16Z","messageCount":2,"participants":["Mike Hommey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"252577","messageId":"1416996767-12672-1-git-send-email-mh@glandium.org","threadId":"38053","inReplyTo":null,"subject":"[PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2014-11-26T10:12:47Z","receivedAt":"2014-11-26T10:12:47Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"An example where this happens is when doing an ls-tree on a tree that\ncontains a commit link. In that case, find_unique_abbrev is called\nto get a non-abbreviated hex sha1, but still, a lookup is done as\nto whether the sha1 is in the repository (which ends up looking for\na loose object in .git/objects), while the result of that lookup is\nnot used when returning a non-abbreviated hex sha1.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n sha1_name.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\nThis hit me on a corner case, where I kind of abuse commit links and\nhave a tree with tens of thousands of them.\n\nWithout the patch:\n  $ time git ls-tree -r HEAD | wc -l\n  114987\n\n  real\t0m4.412s\n  user\t0m1.980s\n  sys\t0m2.480s\n\nWith the patch:\n  $ time git ls-tree -r HEAD | wc -l\n  114987\n\n  real\t0m0.205s\n  user\t0m0.196s\n  sys\t0m0.012s\n\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 5b004f5..cb88170 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -372,10 +372,10 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n \tint status, exists;\n \tstatic char hex[41];\n \n-\texists = has_sha1_file(sha1);\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n \tif (len == 40 || !len)\n \t\treturn hex;\n+\texists = has_sha1_file(sha1);\n \twhile (len < 40) {\n \t\tunsigned char sha1_ret[20];\n \t\tstatus = get_short_sha1(hex, len, sha1_ret, GET_SHA1_QUIETLY);\n-- \n2.1.1\n"},{"id":"252591","messageId":"xmqq7fyhre3b.fsf@gitster.dls.corp.google.com","threadId":"38053","inReplyTo":"1416996767-12672-1-git-send-email-mh@glandium.org","subject":"Re: [PATCH] sha1_name: avoid unnecessary sha1 lookup in find_unique_abbrev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-26T18:50:16Z","receivedAt":"2014-11-26T18:50:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> An example where this happens is when doing an ls-tree on a tree that\n> contains a commit link. In that case, find_unique_abbrev is called\n> to get a non-abbreviated hex sha1, but still, a lookup is done as\n> to whether the sha1 is in the repository (which ends up looking for\n> a loose object in .git/objects), while the result of that lookup is\n> not used when returning a non-abbreviated hex sha1.\n\nMakes sense.  If we are not abbreviating, because the function is\nprepared to give answer for both missing and existing objects, there\nis no point checking the existence.\n\nThanks.\n"}]}