{"thread":{"id":"14846","subject":"[PATCH] Optimize sha1_object_info for loose objects, not concurrent repacks","startedAt":"2008-08-05T20:08:41Z","lastAt":"2008-08-05T20:18:53Z","messageCount":2,"participants":["Steven Grimm","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"86291","messageId":"20080805200841.GA23121@midwinter.com","threadId":"14846","inReplyTo":null,"subject":"[PATCH] Optimize sha1_object_info for loose objects, not concurrent repacks","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2008-08-05T20:08:41Z","receivedAt":"2008-08-05T20:08:41Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"When dealing with a repository with lots of loose objects, sha1_object_info\nwould rescan the packs directory every time an unpacked object was referenced\nbefore finally giving up and looking for the loose object. This caused a lot\nof extra unnecessary system calls during git pack-objects; the code was\nrereading the entire pack directory once for each loose object file.\n\nThis patch looks for a loose object before falling back to rescanning the\npack directory, rather than the other way around.\n\nSigned-off-by: Steven Grimm <koreth@midwinter.com>\n---\n\n\tI discovered this by running strace on a pack-objects that was\n\ttaking especially long to run; it was making more system calls\n\tto scan the pack directory than to do stuff with the loose\n\tobjects, which didn't seem right.\n\n sha1_file.c |    9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e281c14..32e4664 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1929,11 +1929,18 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size\n int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)\n {\n \tstruct pack_entry e;\n+\tint status;\n \n \tif (!find_pack_entry(sha1, &e, NULL)) {\n+\t\t/* Most likely it's a loose object. */\n+\t\tstatus = sha1_loose_object_info(sha1, sizep);\n+\t\tif (status >= 0)\n+\t\t\treturn status;\n+\n+\t\t/* Not a loose object; someone else may have just packed it. */\n \t\treprepare_packed_git();\n \t\tif (!find_pack_entry(sha1, &e, NULL))\n-\t\t\treturn sha1_loose_object_info(sha1, sizep);\n+\t\t\treturn status;\n \t}\n \treturn packed_object_info(e.p, e.offset, sizep);\n }\n-- \n1.6.0.rc1.66.gc78d7\n"},{"id":"86292","messageId":"20080805201853.GG27207@spearce.org","threadId":"14846","inReplyTo":"20080805200841.GA23121@midwinter.com","subject":"Re: [PATCH] Optimize sha1_object_info for loose objects, not concurrent repacks","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-05T20:18:53Z","receivedAt":"2008-08-05T20:18:53Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Steven Grimm <koreth@midwinter.com> wrote:\n> When dealing with a repository with lots of loose objects, sha1_object_info\n> would rescan the packs directory every time an unpacked object was referenced\n> before finally giving up and looking for the loose object. This caused a lot\n> of extra unnecessary system calls during git pack-objects; the code was\n> rereading the entire pack directory once for each loose object file.\n> \n> This patch looks for a loose object before falling back to rescanning the\n> pack directory, rather than the other way around.\n> \n> Signed-off-by: Steven Grimm <koreth@midwinter.com>\n\nHeh.  Cute bug.\n\nACK.\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index e281c14..32e4664 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1929,11 +1929,18 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size\n>  int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)\n>  {\n>  \tstruct pack_entry e;\n> +\tint status;\n>  \n>  \tif (!find_pack_entry(sha1, &e, NULL)) {\n> +\t\t/* Most likely it's a loose object. */\n> +\t\tstatus = sha1_loose_object_info(sha1, sizep);\n> +\t\tif (status >= 0)\n> +\t\t\treturn status;\n> +\n> +\t\t/* Not a loose object; someone else may have just packed it. */\n>  \t\treprepare_packed_git();\n>  \t\tif (!find_pack_entry(sha1, &e, NULL))\n> -\t\t\treturn sha1_loose_object_info(sha1, sizep);\n> +\t\t\treturn status;\n>  \t}\n>  \treturn packed_object_info(e.p, e.offset, sizep);\n>  }\n\n-- \nShawn.\n"}]}